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.

Sign up for GitHub

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

Jump to bottom

Crash in ISOProtectionSystemSpecificHeaderBox::parse with large keyID counts #15027

Merged
merged 1 commit into from
Jun 16, 2023
Merged

Crash in ISOProtectionSystemSpecificHeaderBox::parse with large keyID counts #15027

merged 1 commit into from
Jun 16, 2023

Conversation

aestes
Copy link
Contributor

@aestes aestes commented Jun 15, 2023 •
edited by webkit-early-warning-system
Loading

3b3afcc

Crash in ISOProtectionSystemSpecificHeaderBox::parse with large keyID counts
https://bugs.webkit.org/show_bug.cgi?id=258174
rdar://110412566

Reviewed by Jean-Yves Avenard.

KeyID is 16 bytes, so it's possible for a `pssh` box to specify a `keyIDCount` large enough that the
following keyIDs array cannot be contained by a Vector. To account for this, try to reserve capacity
for `keyIDCount` in m_keyIDs before resizing and return `false` if the reservation fails.

* Source/WebCore/platform/graphics/iso/ISOProtectionSystemSpecificHeaderBox.cpp:
(WebCore::ISOProtectionSystemSpecificHeaderBox::parse):

Canonical link: https://commits.webkit.org/265256@main

22ea988

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe   🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2   🧪 api-mac ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1   🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
  🛠 tv   🧪 mac-AS-debug-wk2
  🛠 tv-sim
✅ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

@aestes aestes self-assigned this Jun 15, 2023
@aestes aestes added the Media Bugs related to the HTML 5 Media elements. label Jun 15, 2023
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 15, 2023 •
edited
Loading

EWS run on previous version of this PR (hash ca22e64)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv ❌ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 16, 2023
@aestes aestes removed the merging-blocked Applied to prevent a change from being merged label Jun 16, 2023
@aestes aestes force-pushed the eng/Crash-in-ISOProtectionSystemSpecificHeaderBoxparse-with-large-keyID-counts branch from ca22e64 to 9d08693 Compare June 16, 2023 20:17
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 16, 2023 •
edited
Loading

EWS run on previous version of this PR (hash 9d08693)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios   🛠 mac ✅ 🛠 wpe ⏳ 🛠 wincairo
  🧪 bindings   🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2
  🧪 webkitperl   🧪 ios-wk2   🧪 api-mac   🛠 gtk
  🧪 ios-wk2-wpt   🧪 mac-wk1   🧪 gtk-wk2
⏳ 🛠 🧪 jsc   🧪 api-ios   🧪 mac-wk2   🧪 api-gtk
⏳ 🛠 tv ⏳ 🧪 mac-AS-debug-wk2
⏳ 🛠 tv-sim   🧪 mac-wk2-stress
⏳ 🛠 watch
  🛠 watch-sim

@aestes aestes force-pushed the eng/Crash-in-ISOProtectionSystemSpecificHeaderBoxparse-with-large-keyID-counts branch from 9d08693 to 22ea988 Compare June 16, 2023 20:19
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 16, 2023 •
edited
Loading

EWS run on current version of this PR (hash 22ea988)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe   🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2   🧪 api-mac ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1   🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
  🛠 tv   🧪 mac-AS-debug-wk2
  🛠 tv-sim
✅ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

@aestes aestes added the merge-queue Applied to send a pull request to merge-queue label Jun 16, 2023
@aestes
Crash in ISOProtectionSystemSpecificHeaderBox::parse with large keyID…
3b3afcc 
… counts

https://bugs.webkit.org/show_bug.cgi?id=258174
rdar://110412566

Reviewed by Jean-Yves Avenard.

KeyID is 16 bytes, so it's possible for a `pssh` box to specify a `keyIDCount` large enough that the
following keyIDs array cannot be contained by a Vector. To account for this, try to reserve capacity
for `keyIDCount` in m_keyIDs before resizing and return `false` if the reservation fails.

* Source/WebCore/platform/graphics/iso/ISOProtectionSystemSpecificHeaderBox.cpp:
(WebCore::ISOProtectionSystemSpecificHeaderBox::parse):

Canonical link: https://commits.webkit.org/265256@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Crash-in-ISOProtectionSystemSpecificHeaderBoxparse-with-large-keyID-counts branch from 22ea988 to 3b3afcc Compare June 16, 2023 21:01
@webkit-commit-queue
Copy link
Collaborator

Committed 265256@main (3b3afcc): https://commits.webkit.org/265256@main

Reviewed commits have been landed. Closing PR #15027 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 3b3afcc into WebKit:main Jun 16, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jyavenard jyavenard jyavenard approved these changes

Assignees

@aestes aestes

Labels
Media Bugs related to the HTML 5 Media elements.
Projects
None yet
Milestone
No milestone
5 participants
@aestes @webkit-early-warning-system @webkit-commit-queue @jyavenard @webkit-ews-buildbot