OpenSSL undefined pointer arithmetic

2016.06.09
Credit: Alexander
Risk: Medium
Local: No
Remote: Yes
CWE: CWE-190


CVSS Base Score: 7.5/10
Impact Subscore: 6.4/10
Exploitability Subscore: 10/10
Exploit range: Remote
Attack complexity: Low
Authentication: No required
Confidentiality impact: Partial
Integrity impact: Partial
Availability impact: Partial

Hi, More off Twitter: <guidovranken> @solardiz Here is another very recent OpenSSL CVE that hasnt been released officially yet https://github.com/openssl/openssl/commit/a004e72b95835136d3f1ea90517f706c24c03da7 | Avoid some undefined pointer arithmetic | | A common idiom in the codebase is: | | if (p + len > limit) | { | return; /* Too long */ | } | | Where "p" points to some malloc'd data of SIZE bytes and | limit == p + SIZE | | "len" here could be from some externally supplied data (e.g. from a TLS | message). | | The rules of C pointer arithmetic are such that "p + len" is only well | defined where len <= SIZE. Therefore the above idiom is actually | undefined behaviour. | | For example this could cause problems if some malloc implementation | provides an address for "p" such that "p + len" actually overflows for | values of len that are too big and therefore p + len < limit! | | Issue reported by Guido Vranken. | | CVE-2016-2177 The commit message above gives pointer wraparound as an example of when and how this UB could manifest itself, but I think even more likely is that an optimizing C compiler would remove the check because it can't be reliably true (it can be either false or UB). A valid pointer is at most one element beyond the end of an object, so in the example given above "p + len > limit" is never reliably true. In the actual code being patched, there are different instances of the problem, and some of the checks look like they're effectively ">= limit" rather than "> limit". Those ">=" checks are more lucky, as they can't be completely removed (but can still misbehave if the pointer advances further). In fact, there are so many instances that someone should re-review the patch and possibly look for even more instances of the problem (maybe in an automated way different from what might have been used so far). The commit has "Reviewed-by: Rich Salz", which is great, but I think it needs more eyes than the committer's and one other person's. Alexander

References:

https://github.com/openssl/openssl/commit/a004e72b95835136d3f1ea90517f706c24c03da7
http://seclists.org/oss-sec/2016/q2/500


Vote for this issue:
50%
50%


 

Thanks for you vote!


 

Thanks for you comment!
Your message is in quarantine 48 hours.

Comment it here.


(*) - required fields.  
{{ x.nick }} | Date: {{ x.ux * 1000 | date:'yyyy-MM-dd' }} {{ x.ux * 1000 | date:'HH:mm' }} CET+1
{{ x.comment }}

Copyright 2024, cxsecurity.com

 

Back to Top