Skip to content

CFStringGetCStringPtr truncates on internal NUL byte #5200

Open
@madsmtm

Description

@madsmtm

CFStringGetCStringPtr is meant as an optimization to allow users to avoid a copy when the string is known to be valid ASCII, UTF-8 or similar 8-bit encoding.

It has a flaw though: It does not check whether the string contains interior NUL bytes. Consider the following Swift code (tested on macOS 14.7.4) (not specific to Swift, the problem is present in plain Objective-C too):

import CoreFoundation

// All of Swift.String, NSString and CFString support strings with interior NUL bytes.
let s = "A string with a \0 <- NUL right there"
print(s) // Prints the full string, i.e. preserves the content after the NUL byte.
print(s.count) // Prints 36 as expected.

func cf_roundtrip(_ s: String) -> String? {
    let cfstr = s as! CFString
    guard let ptr = CFStringGetCStringPtr(cfstr, CFStringBuiltInEncodings.UTF8.rawValue) else {
        return nil
    }
    return String(cString: ptr)
}

// Most strings can be round-tripped through `CFStringGetCStringPtr`, and if it can't, will return `nil`.
print(cf_roundtrip("Hello World!") as Any)           // Prints `Optional("Hello World!")`
print(cf_roundtrip("Contains non-ASCII: 😀") as Any) // Prints `nil`

// Round-tripping strings with interior NUL bytes through `CFStringGetCStringPtr` doesn't work correctly though:
print(cf_roundtrip(s) as Any) // Prints `Optional("A string with a ")`
// !!! The string was truncated, should have returned `nil` instead!

That is, CFStringGetCStringPtr ends up returning the string pointer, but because of consumers assuming that the internal NUL byte is the final NUL, it silently truncates the rest of the string. A mitigation here would be to only use ASCII and to check CFStringGetLength as done in 860956a and 8422c1a when Swift.String itself was affected by this bug in the past, but I suspect only the vast minority of people will find that solution.

I actually found this by using CFURLCreateWithBytes with an interior NUL byte, which makes it unexpectedly fail because of this check that was added in newer versions.

I suspect this is potentially a security issue (see e.g. CWE-158 / CWE-626 and RUSTSEC-2021-0123). Semi related to #5164.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions