Open Bug 1272020 Opened 8 years ago Updated 4 months ago

Undefined behavior in fix for bug 1140537

Categories

(Core :: XML, defect)

Product:
Core
Component:
XML
Type:
defect

Tracking

()

Tracking Flags:
Tracking Status
firefox49 --- affected

People

(Reporter: mccr8, Unassigned)

References

Details

(Keywords: csectype-undefined)

Attachments

(1 obsolete file) 

It sounds like checking overflow by seeing if the addition of two positive values is negative is undefined behavior, and a sufficiently smart compiler is then allowed to remove the check. It sounds like newer versions of GCC can do this sometimes, though I'm not sure if they do it in this particular case.

This was reported to me on Twitter: https://twitter.com/spun_off/status/730281375828480001

Anyways, we should probably use CheckedInt instead.
Keywords: csectype-undefined
Seems like it's not an issue yet:

> yes apparently the only reason expat compiled with GCC is safe is that GCC doesn't infer yet that bufferSize is positive before loop
See also bug 1031653 (not an actual See Also because those are public, although this one was discussed in public so that may not matter).
Blocks: CVE-2015-2716
Group: dom-core-security

(In reply to Andrew McCreight [:mccr8] (Back Jan 3) from comment #0)

This was reported to me on Twitter:
https://twitter.com/spun_off/status/730281375828480001

Upstream libexpat has a fix for that in commit https://github.com/libexpat/libexpat/commit/f0bec73b018caa07d3e75ec8dd967f3785d71bde that is included with releases >=2.2.0.

Severity: normal → S3
Attachment #9385380 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.