search

webrtc-sdk / webrtc

BSD 3-Clause "New" or "Revised" License
225 stars 89 forks source link

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

Closed selfisekai closed 11 months ago

selfisekai commented 11 months ago

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

yuroller commented 8 months ago

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 commented 8 months ago

libvpx seems to read the version number from CHANGELOG, so I think this issue has been fixed old: https://chromium.googlesource.com/webm/libvpx/+/27171320f5e36f7b18071bfa1d9616863ca1b4e8/CHANGELOG new: https://chromium.googlesource.com/webm/libvpx/+/7aaffe2df4c9426ab204a272ca5ca52286ca86d4/CHANGELOG

yuroller commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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.