Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CBOR decode crashes #92

Closed
marius-se opened this issue Mar 29, 2023 · 2 comments
Closed

CBOR decode crashes #92

marius-se opened this issue Mar 29, 2023 · 2 comments

Comments

@marius-se
Copy link

When passing random data to CBOR.decode() the entire app just crashes with an EXC_BAD_ACCESS (code=2, address=0x16f603f60)

Is that intended? If so, how can I add a safety check?

Example:

import Foundation
import SwiftCBOR

@main
public struct CBORCrash {
    public static func main() throws {
        for _ in 1...50 {
            let length = Int.random(in: 1...1_000_000)
            let randomData = Data([UInt8](repeating: UInt8.random(in: 0...255), count: length))
            let result = try CBOR.decode([UInt8](randomData))
        }
    }
}
@hamchapman
Copy link
Collaborator

This can be a tricky one, especially with the sort of pathological case you've got in your code above.

For example, if you take this code and run it as a test:

func testDecodeBadBytes() {
    let length = 10000
    let randomUInt: UInt8 = 202
    let randomData = Data([UInt8](repeating: randomUInt, count: length))
    let result = try? CBOR.decode([UInt8](randomData))
    XCTAssertNotNil(result)
}

then you'll get a crash like the one you described. This is because 202 represents the initial byte of a tagged value, and with that value being repeated the decoded keeps on trying to decode nested tagged values. Eventually something goes wrong, which I'm not exactly sure about yet, and you get the crash.

If you take that example from above and only change the value of length to be 1000 instead then I don't get a crash and instead the value of result is nil, because the decoding failed.

I don't know what the best approach would be here to try and avoid the crashes. It would need more investigation and I don't currently have time for that I'm afraid.

@dimitribouniol
Copy link
Contributor

I have a fix in #101, verified using both suggested tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants