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

roll libvpx to include fix for CVE-2023-5217 #98

Merged
merged 1 commit into from
Sep 28, 2023

Conversation

selfisekai
Copy link

this rolls these 2 CLs in:
https://chromium-review.googlesource.com/c/webm/libvpx/+/4888549
https://chromium-review.googlesource.com/c/webm/libvpx/+/4888550

vuln described so far just here: https://chromereleases.googleblog.com/2023/09/stable-channel-update-for-desktop_27.html

[$NA][1486441] High CVE-2023-5217: Heap buffer overflow in vp8 encoding in libvpx. Reported by Clément Lecigne of Google's Threat Analysis Group on 2023-09-25

https://nvd.nist.gov/vuln/detail/CVE-2023-5217

Copy link
Member

@cloudwebrtc cloudwebrtc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@cloudwebrtc cloudwebrtc merged commit fcab26b into webrtc-sdk:m114_release Sep 28, 2023
@yuroller
Copy link

it seems that binaries released in repos:
https://github.com/webrtc-sdk/android
https://github.com/webrtc-sdk/Specs
still contains the old libvpx.
for android:
$ strings libjingle_peerconnection_so.so | grep '1.13'
WebM Project VP9 Encoder v1.13.0-243-g27171320f
WebM Project VP8 Encoder v1.13.0-243-g27171320f
WebM Project VP9 Decoder v1.13.0-243-g27171320f
WebM Project VP8 Decoder v1.13.0-243-g27171320f

@cloudwebrtc
Copy link
Member

@yuroller
Copy link

The commit in this repo specifies (correctly) libvpx 7aaffe2df, but somehow (stale files?) the builds on android and Spec repos are not using this version of libvpx, but the old 27171320f.
If you download the compiled binaries from the android and Specs repo and search for the libvpx version string, you see the old commit 27171320f.

android:
$ strings libjingle_peerconnection_so.so | grep '1.13'
WebM Project VP9 Encoder v1.13.0-243-g27171320f
WebM Project VP8 Encoder v1.13.0-243-g27171320f
WebM Project VP9 Decoder v1.13.0-243-g27171320f
WebM Project VP8 Decoder v1.13.0-243-g27171320f

spec:
$ strings WebRTC | grep '1.13'
WebM Project VP8 Encoder v1.13.0-243-g27171320f
WebM Project VP8 Decoder v1.13.0-243-g27171320f
WebM Project VP9 Encoder v1.13.0-243-g27171320f
WebM Project VP9 Decoder v1.13.0-243-g27171320f

@davidliu
Copy link
Contributor

davidliu commented Jan 15, 2024

So what's happening here is that the particular version string you're seeing here is controlled by the parent repo for src/third_party:

https://chromium.googlesource.com/chromium/src/third_party/+/4f8bf4c6885/libvpx/source/config/vpx_version.h

(Hash referenced from: https://github.com/webrtc-sdk/webrtc/blob/8c9aa75abf1aaa4bd79d5aaa70fc000565b9b564/DEPS#L66C8-L66C8)

However, this version string is independent from the actual code, which is in the src/third_party/libvpx/source/libvpx repo (updated by this PR). It would be nice to update this string as well to clarify the issue, but I'm not sure if there's a way to specifically update the third_party repo cleanly to do so (without opening yet another fork for this specific purpose).

Did a double check locally, and gclient sync is properly updating the libvpx source to 7aaffe2df, while the version string stays the same.


We'll likely be pulling from upstream in the coming months, which should resolve any remaining confusion.

@yuroller
Copy link

So if I understand your explanation, the actual code that is built contains the new revision of libvpx, but due to a dependency from a repo that referenced the old revision of libvpx, the string inserted into the binaries is extracted (wrongly) from this dependent repo: it is just a "cosmetic" issue, because the compiled code is correct.
Thanks for the time and for taking care of this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants