Closed Bug 1459722 Opened 3 years ago Closed 3 years ago

Undefined behavior in zxx_stream::zxx_stream()

Categories

(Core :: mozglue, defect)

59 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox60 --- wontfix
firefox61 --- wontfix
firefox62 --- fixed

People

(Reporter: mozillabugs, Assigned: glandium)

Details

Attachments

(1 file)

zxx_stream::zxx_stream() (mozglue\linker\zip.h) initializes itself in part using memset():

   40: memset (this, 0, sizeof(z_stream));

where |z_stream| is a base class of |zxx_stream|. This code invokes undefined behavior, since it assumes that the base class subobject resides at |this|, which contravenes C++11 s.10:

------
[t]he order in which the base class subobjects are allocated in the most derived object (1.8) is unspecified."
------

zxx_stream() appears to be used in Android builds only.
Flags: sec-bounty?
Group: core-security → dom-core-security
Mike, can you please take a look at this and weigh in on the severity?
Practically speaking, it doesn't matter. The worse that could happen is the allocator field being overwritten with zeroes, which would only mean zlib would be using new/delete instead of the given allocator. But in practice that doesn't happen anyways.
Flags: needinfo?(mh+mozilla)
And in fact, the only thing that was using the StaticAllocator was removed in bug 1376704, so a lot of this can actually be removed.
I have a patch to remove the whole class. Can this bug be unflagged as confidential so that I can push it to mozreview?
Group: dom-core-security
Assignee: nobody → mh+mozilla
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: sec-bounty?
Comment on attachment 8974608 [details]
Bug 1459722 - Remove zxx_stream.

https://reviewboard.mozilla.org/r/242982/#review248876

WFM.
Attachment #8974608 - Flags: review?(nfroyd) → review+
https://hg.mozilla.org/mozilla-central/rev/ba2417447483
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in before you can comment on or make changes to this bug.