Fix a few more secure decoding issues #13886
Merged
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 ed6ea78)
|
achristensen07
force-pushed
the
eng/Fix-a-few-more-secure-decoding-issues
branch
from
ed6ea78
to
3c8b180
Compare
|
EWS run on previous version of this PR (hash 3c8b180)
|
achristensen07
force-pushed
the
eng/Fix-a-few-more-secure-decoding-issues
branch
from
3c8b180
to
abd428d
Compare
|
EWS run on previous version of this PR (hash abd428d)
|
whsieh
approved these changes
May 15, 2023
| if (auto mutableArray = dynamic_objc_cast<NSMutableArray>(object); mutableArray && rewriteMutableArray) | ||
| return adoptNS([mutableArray copy]).autorelease(); | ||
| if (rewriteMutableArray) { | ||
| if (auto mutableArray = dynamic_objc_cast<NSMutableArray>(object); mutableArray) |
There was a problem hiding this comment.
Suggested change
| if (auto mutableArray = dynamic_objc_cast<NSMutableArray>(object); mutableArray) | |
| if (auto mutableArray = dynamic_objc_cast<NSMutableArray>(object)) |
| if (auto mutableData = dynamic_objc_cast<NSMutableData>(object); mutableData && rewriteMutableData) | ||
| return adoptNS([mutableData copy]).autorelease(); | ||
| if (rewriteMutableData) { | ||
| if (auto mutableData = dynamic_objc_cast<NSMutableData>(object); mutableData) |
There was a problem hiding this comment.
Suggested change
| if (auto mutableData = dynamic_objc_cast<NSMutableData>(object); mutableData) | |
| if (auto mutableData = dynamic_objc_cast<NSMutableData>(object)) |
| if (auto mutableDict = dynamic_objc_cast<NSMutableDictionary>(object); mutableDict && rewriteMutableDictionary) | ||
| return adoptNS([mutableDict copy]).autorelease(); | ||
| if (rewriteMutableDictionary) { | ||
| if (auto mutableDict = dynamic_objc_cast<NSMutableDictionary>(object); mutableDict) |
There was a problem hiding this comment.
Suggested change
| if (auto mutableDict = dynamic_objc_cast<NSMutableDictionary>(object); mutableDict) | |
| if (auto mutableDict = dynamic_objc_cast<NSMutableDictionary>(object)) |
| } | ||
|
|
||
| if (rewriteMutableString) { | ||
| if (auto mutableString = dynamic_objc_cast<NSMutableString>(object); mutableString) |
There was a problem hiding this comment.
Suggested change
| if (auto mutableString = dynamic_objc_cast<NSMutableString>(object); mutableString) | |
| if (auto mutableString = dynamic_objc_cast<NSMutableString>(object)) |
achristensen07
force-pushed
the
eng/Fix-a-few-more-secure-decoding-issues
branch
from
abd428d
to
cbdf304
Compare
|
EWS run on current version of this PR (hash cbdf304)
|
achristensen07
added
the
unsafe-merge-queue
https://bugs.webkit.org/show_bug.cgi?id=256790 rdar://109286881 Reviewed by Wenson Hsieh. Reports indicate DDScannerResult can contain NSMutableStrings, which fail to decode in strict mode when expecting an NSString. Fix this by doing the same transformation from mutable to not mutable on the encoding side that we do elsewhere. Reports also indicate that NSURLRequest can contain mutable plist types like NSMutableURLRequest can. To be conservative, add the same allowed mutable plist types when decoding the two classes. The soft linking code for PAL::isDataDetectorsCoreFrameworkAvailable already effectively caches the result of dlsym, so making our own cache is redundant. Same with the other similar caches. Also, as a slight perf optimization, check the bools like rewriteMutableString first before calling dynamic_objc_cast since the former is often false, the latter is more expensive, and both need to be true to enter the condition. * Source/WebKit/Shared/Cocoa/ArgumentCodersCocoa.mm: (-[WKSecureCodingArchivingDelegate archiver:willEncodeObject:]): (-[WKSecureCodingArchivingDelegate init]): (IPC::encodeSecureCodingInternal): (IPC::shouldEnableStrictMode): (IPC::decodeSecureCodingInternal): Canonical link: https://commits.webkit.org/264079@main
webkit-commit-queue
force-pushed
the
eng/Fix-a-few-more-secure-decoding-issues
branch
from
cbdf304
to
b17879d
Compare
|
Committed 264079@main (b17879d): https://commits.webkit.org/264079@main Reviewed commits have been landed. Closing PR #13886 and removing active labels. |
webkit-commit-queue
removed
the
unsafe-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.
b17879d
cbdf304
🧪 ios-wk2-wpt🧪 gtk-wk2🧪 api-ios