Status

()

defect
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: tjr, Assigned: RyanVM)

Tracking

({csectype-bounds, sec-moderate})

Trunk
mozilla54
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox-esr45- wontfix, firefox51 wontfix, firefox52+ fixed, firefox-esr5252+ fixed, firefox53+ fixed, firefox54+ fixed)

Details

(Whiteboard: [post-critsmash-triage][adv-main52-] [third-party-lib-audit])

Attachments

(1 attachment)

This is a (semi-)automated bug making you aware that there is an available upgrade for an embedded third-party library.

libogg is currently at version 1.3.0 in mozilla-central, and the latest version of the library released is 1.3.2 

I fetched the latest version of the library from https://xiph.org/downloads/

You can leave this bug open, and it will be updated if a newer version of the library becomes available. If you close it as WONTFIX, please indicate if you do not wish to receive any future bugs upon new releases of the library.
----
Additionally, while examining the library to figure out how to track its version,  it seems like there may be two commits that are security relevent, hence this is flagged as such.

--------
commit 40ef08179765ad1595bd83b83bd85218b6773fb3
Author: Monty <xiphmont@xiph.org>
Date:   Thu Apr 24 20:59:56 2014 +0000

    Correct oggpack_writecopy bug reported by Ian Nartowicz: Integer
    overflow checking in oggpack_writecopy_helper got the reallocation
    size test condition backwards and so would error out when it needed to
    expand the destination's internal buffer.

    At the same time, do preexpansion of both aligned and unaligned copies
    to avoid possible heap thrashing in the unaligned case.

    Add black and glass box unit tests for oggpack_writecopy and
    oggpackB_writecopy.
---------
commit 85dbd8d9ed3d9af12dd69af4801e0620158eb609
Author: Tim Terriberry <tterribe@xiph.org>
Date:   Tue Jan 8 16:29:56 2013 +0000

    Guard against very large packets.

    Their size could overflow a long (especially on, e.g., Win64, where
     they could still fit in memory).
---------
Oh, to get the current source you'll need to git clone https://git.xiph.org/ogg.git/
https://treeherder.mozilla.org/#/jobs?repo=try&revision=07f4a0b50c14fff52e2efbfdcfadad54da6bcd47

Will request review after the Try run finishes up.
Assignee: nobody → ryanvm
Status: NEW → ASSIGNED
Would be good to get a sense of how s-s this is so we can decide if backporting is warranted or not.
Comment on attachment 8837338 [details] [diff] [review]
update libogg to version 1.3.2

Had to do another Try push because my first one was on top of a broken parent rev.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4ca0dd5cef1f17fb07a8bd1ac41d4394c7b63490&group_state=expanded

Ralph, can you please comment on the potential security implications of this bug in addition to doing the review? Thanks! :)
Attachment #8837338 - Flags: review?(giles)
Comment on attachment 8837338 [details] [diff] [review]
update libogg to version 1.3.2

Review of attachment 8837338 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for doing the update. The import looks correct.

As far as the security questions:

- The `oggpack_write()` bugfix doesn't affect Firefox; it's only used by muxers.
- The 'large packet guard' change does affect a code path we use. In theory the allocation size overflow could allow invalid writes, but only after the user downloads >2GB of malicious data. I don't think we would buffer that much without any playback progress, but I'm not sure. I believe it also only affects win64, where `long` is a smaller size than `ptr`.

My feeling would be that should uplift this to aurora--and maybe esr52--but it's not necessary to propagate it to beta.
Attachment #8837338 - Flags: sec-approval?
Attachment #8837338 - Flags: review?(giles)
Attachment #8837338 - Flags: review+
Group: core-security → media-core-security
Comment on attachment 8837338 [details] [diff] [review]
update libogg to version 1.3.2

Approval Request Comment
[Feature/Bug causing the regression]: N/A
[User impact if declined]: Possible win64 sec issues under some circumstances
[Is this code covered by automated tests?]: We have automated tests for ogg playback, yes.
[Has the fix been verified in Nightly?]: N/A
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Low-risk
[Why is the change risky/not risky?]: Version 1.3.2 is nearly two years old already without known regressions caused by these changes.
[String changes made/needed]: None
Attachment #8837338 - Flags: approval-mozilla-beta?
Attachment #8837338 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/6ea939aba894
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment on attachment 8837338 [details] [diff] [review]
update libogg to version 1.3.2

libogg bugfix update, aurora53+, beta52+
Attachment #8837338 - Flags: approval-mozilla-beta?
Attachment #8837338 - Flags: approval-mozilla-beta+
Attachment #8837338 - Flags: approval-mozilla-aurora?
Attachment #8837338 - Flags: approval-mozilla-aurora+
Group: media-core-security → core-security-release
Flags: qe-verify-
Whiteboard: [post-critsmash-triage]
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main52-]
No longer blocks: 1325608
Whiteboard: [post-critsmash-triage][adv-main52-] → [post-critsmash-triage][adv-main52-] [third-party-lib-audit]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.