SDK-469 Fix CodeQL string length conflation in deep link check#1071
Open
sumeruchat wants to merge 2 commits into
Open
SDK-469 Fix CodeQL string length conflation in deep link check#1071sumeruchat wants to merge 2 commits into
sumeruchat wants to merge 2 commits into
Conversation
CodeQL flagged a String-length conflation (CWE-135) in DeepLinkManager.isIterableDeepLink: the search range for NSRegularExpression was built with NSMakeRange(0, urlString.count), mixing a Swift String grapheme-cluster count with NSRegularExpression's NSString/UTF-16 semantics. For URLs containing non-BMP characters (e.g. emoji) the two measures diverge, producing a range that under-covers the string and can cause the deep-link match to be missed. - Build the NSRange from the bridged NSString.length so the range is expressed in the same UTF-16 coordinates NSRegularExpression uses. - Add two regression tests: a URL with a long non-BMP prefix before the "/a/..." segment (fails against the old String.count range, passes with the fix), and a non-BMP URL without a deep-link path (stays false). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1071 +/- ##
==========================================
+ Coverage 71.22% 71.28% +0.05%
==========================================
Files 112 112
Lines 9365 9370 +5
==========================================
+ Hits 6670 6679 +9
+ Misses 2695 2691 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Re-run CI after an unrelated flaky failure in InboxTests.testInboxAndInAppCallbacksTogether (inbox-message-count race on the first notification callback; passes 4/4 locally). No code change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What
Fixes a CodeQL-flagged String-length conflation (CWE-135) in
DeepLinkManager.isIterableDeepLink. TheNSRegularExpressionsearch range was built fromurlString.count(a SwiftStringgrapheme-cluster count) whileNSRegularExpressionoperates onNSString/UTF-16 coordinates. For URLs with non-BMP characters (e.g. emoji), those measures diverge and the range under-covers the string, so a valid deep link can be missed. Ticket: SDK-469.Changes
DeepLinkManager.swift: build theNSRangefrom the bridgedNSString.lengthinstead ofString.count, so the range matches the UTF-16 coordinatesNSRegularExpressionuses. Matches the ticket's prescribed fix and CodeQL's own recommended example.DeepLinkTests.swift: two regression tests — a URL with a long non-BMP prefix before the/a/...segment (this case returnsfalseunder the oldString.countrange andtruewith the fix), and a non-BMP URL without a deep-link path (staysfalse).Impact
String→NSStringbridge per call.Testing
How to test:
./agent_test.sh DeepLinkTests— 11/11 pass, including the two new SDK-469 cases.DeepLinkManager.swift.https://links.iterable.com/👍🏿…/a/<id>is now correctly recognized as an Iterable deep link.