Limit untrusted allocations when decoding Vectors to 1MB #9699
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
achristensen07
added
the
WebKit Misc.
|
EWS run on previous version of this PR (hash 3c59e4d)
|
kkinnunen-apple
approved these changes
Feb 6, 2023
There was a problem hiding this comment.
Good catch.
This should be testable, probably in some test similar to ArgumentCoderSpanTest but probably it needs a new fixture.. The test would be one where the encode encodes size_t big value and decoder decodes Vector and the test would test that decoder decodes nullopt instead of a crash.
achristensen07
force-pushed
the
eng/Limit-untrusted-allocations-when-decoding-Vectors-to-1MB
branch
from
3c59e4d
to
55a3be2
Compare
|
EWS run on current version of this PR (hash 55a3be2)
|
achristensen07
added
the
merge-queue
https://bugs.webkit.org/show_bug.cgi?id=251804 Reviewed by Kimmo Kinnunen. 257725@main introduced a performance improvement where we only allocate exactly as much memory as we need once when decoding a Vector. This is wonderful, but it introduced allocation based on size from an untrusted source, making it so any message that sends a Vector can be used to send a very large size_t and crash the other process. In this PR I get the best of both worlds: if the total allocation size is less that 1MB then we do the fast and efficient thing, but if it is more than 1MB we do the safe thing. * Source/WebKit/Platform/IPC/ArgumentCoders.h: Canonical link: https://commits.webkit.org/259917@main
webkit-early-warning-system
force-pushed
the
eng/Limit-untrusted-allocations-when-decoding-Vectors-to-1MB
branch
from
55a3be2
to
23f2542
Compare
|
Committed 259917@main (23f2542): https://commits.webkit.org/259917@main Reviewed commits have been landed. Closing PR #9699 and removing active labels. |
webkit-commit-queue
removed
the
merge-queue
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
23f2542
55a3be2
🛠 ios🛠 mac🛠 🧪 win🛠 mac-AS-debug🛠 gtk🛠 wincairo🧪 ios-wk2🧪 api-mac🧪 gtk-wk2🧪 api-ios🧪 mac-wk1🧪 api-gtk🛠 tv🧪 mac-wk2🧪 mac-AS-debug-wk2🧪 mac-wk2-stress🛠 watch-sim