String / NSString length mismatch in PhoneNumberParser.parsePrefixAsIdd (potential out-of-bounds)

[MarcGarciaSunweb]

Describe the bug
PhoneNumberParser.parsePrefixAsIdd(_:, iddPattern:) mixes Swift String character counts with NSString UTF-16 offsets. Specifically, it derives an offset using String.count and then uses that value as the from: index in NSString.substring(from:), which expects a UTF-16 code unit offset. For certain Unicode sequences, String.count and NSString.length are not equivalent, which can lead to incorrect slicing and, in some cases, out-of-bounds behavior.

GitHub Advanced Security flags this as a high-severity code scanning security alert: using a String length in an NSString context may cause unexpected behavior, including potential buffer overwrites/out-of-bounds.

Even though this isn’t an issue in practice because the phone number is validated before parsePrefixAsIdd is called (preventing Unicode characters from being treated as valid input), it’s still worth fixing to ensure correctness and avoid potential future issues.

Where
File: PhoneNumberParser.swift
Method: parsePrefixAsIdd(_:, iddPattern:)

Why this is a problem

• String.count measures extended grapheme clusters (user-perceived characters).
• NSString APIs (including substring(from:), NSRange) use UTF-16 code unit offsets.
• For characters such as emoji with modifiers (e.g. 👍🏿), combining marks, or other multi-scalar grapheme clusters, these measurements can diverge.

Suggested fix

1. Do not use matchedString.count to compute the end position for NSString.substring(from:).
   Instead, use the match’s NSRange to compute a UTF-16 offset:

• let matchEnd = matched.range.location + matched.range.length
• guard matchEnd <= (number as NSString).length else { return false }
• let remainString = (number as NSString).substring(from: matchEnd)

2. Avoid applying NSRange directly to Swift String slicing. Convert using Range(_:in:):

• if let range = Range(firstMatch.range, in: remainString) { ... }

Proposed patch

func parsePrefixAsIdd(_ number: inout String, iddPattern: String) -> Bool {
    if self.regex.stringPositionByRegex(iddPattern, string: number) == 0 {
        do {
            guard let matched = try regex.regexMatches(iddPattern, string: number).first else {
                return false
            }

            let matchEnd = matched.range.location + matched.range.length
            let nsNumber = number as NSString
            guard matchEnd <= nsNumber.length else { return false }

            let remainString = nsNumber.substring(from: matchEnd)

            let capturingDigitPatterns = try NSRegularExpression(
                pattern: PhoneNumberPatterns.capturingDigitPattern,
                options: .caseInsensitive
            )
            let matchedGroups = capturingDigitPatterns.matches(in: remainString)

            if let firstMatch = matchedGroups.first,
               let range = Range(firstMatch.range, in: remainString) {
                let digitMatched = String(remainString[range])
                if !digitMatched.isEmpty {
                    let normalizedGroup = self.regex.stringByReplacingOccurrences(
                        digitMatched,
                        map: PhoneNumberPatterns.allNormalizationMappings
                    )
                    if normalizedGroup == "0" {
                        return false
                    }
                }
            }

            number = remainString
            return true
        } catch {
            return false
        }
    }
    return false
}

Impact

• Fixes a correctness issue in Unicode handling.
• Removes a class of potential out-of-bounds bugs flagged by static analysis.
• Keeps behavior consistent with Apple guidance: don’t mix String.count and NSString.length/UTF-16 offsets.