Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Condition always false in x509_get_subject_alt_name #2802

Closed
irwir opened this issue Aug 22, 2019 · 2 comments · Fixed by #2855
Closed

Condition always false in x509_get_subject_alt_name #2802

irwir opened this issue Aug 22, 2019 · 2 comments · Fixed by #2855
Labels
bug component-x509 help-wanted This issue is not being actively worked on, but PRs welcome.

Comments

@irwir
Copy link
Contributor

irwir commented Aug 22, 2019

Comparing with zero could be slightly more efficient, but also it is easier to understand.

https://github.com/ARMmbed/mbedtls/blob/beec142010cd9f3a967a9a3a7b407b51841b542e/library/x509_crt.c#L648
It might be guessed that the intention was to check end>=*p, but the values are pointers, and thus unsigned.
In pointer arithmetics anything less than 1 is nothing but 0.
Hence the condition means end == *p,

In any case, the loop header ensures that *p < end.

@Patater Patater added the help-wanted This issue is not being actively worked on, but PRs welcome. label Aug 29, 2019
irwir added a commit to irwir/mbedtls that referenced this issue Sep 21, 2019
@k-stachowiak
Copy link
Contributor

The check is in fact redundant, but I disagree with the explanation. end - *p is of type ptrdiff_t which is a signed type. It could be negative if *p went beyond end.
That will not happen, but not because of the pointer arithmetic. It won't happen because of the loop condition that you have pointed out.

@irwir
Copy link
Contributor Author

irwir commented Oct 24, 2019

@k-stachowiak
Thanks for looking at the issue.

end - *p is of type ptrdiff_t which is a signed type.

You are right here. Idea of treating pointers as unsigned remained from old times; now this is implementation defined.
By the way, VS compiler would use signed comparison in case of (end - *p) > 1, but unsigned if the condition is end > *p + 1

It could be negative if *p went beyond end.

Or if the difference is above maximum positive value for prtdiff_t type?
That is why it might be better to avoid expressions such as end - *p (quite a few of these are in asn1parse.c).

irwir added a commit to irwir/mbedtls that referenced this issue Apr 18, 2020
irwir added a commit to irwir/mbedtls that referenced this issue Apr 18, 2020
irwir added a commit to irwir/mbedtls that referenced this issue Apr 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug component-x509 help-wanted This issue is not being actively worked on, but PRs welcome.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants