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

Crash in ChaCha20Poly1305.encrypt #86

Closed
gbrooker opened this issue Apr 2, 2019 · 3 comments
Closed

Crash in ChaCha20Poly1305.encrypt #86

gbrooker opened this issue Apr 2, 2019 · 3 comments

Comments

@gbrooker
Copy link
Contributor

gbrooker commented Apr 2, 2019

There is a crashing bug in the packet encryption code, if the buffer length is 495 or 496 bytes long.

The crash can be demonstrated by adding the following test case to CryptographerTests

    func testCrash() {

        let crashMessage = "6174223a22737472696e67222c2274797065223a223230222c227065726d73223a5b227072225d7d2c7b2274797065223a223231222c22696964223a352c2276616c7565223a224861726d6f6e7920487562204163746976697479222c226465736372697074696f6e223a224d6f64656c222c227065726d73223a5b227072225d2c22666f726d6174223a22737472696e67227d2c7b2274797065223a223532222c22696964223a362c226465736372697074696f6e223a224669726d77617265205265766973696f6e222c22666f726d6174223a22737472696e67222c227065726d73223a5b227072225d2c2276616c7565223a22342e31352e323530227d2c7b226465736372697074696f6e223a224964656e74696679222c227065726d73223a5b227077225d2c22666f726d6174223a22626f6f6c222c2274797065223a223134222c22696964223a377d5d2c22696964223a317d2c7b22636861726163746572697374696373223a5b7b2276616c7565223a66616c73652c227065726d73223a5b227072222c227077222c226576225d2c226465736372697074696f6e223a22506f776572205374617465222c22696964223a392c22666f726d6174223a22626f6f6c222c2274797065223a223235227d5d2c22696964223a382c2274797065223a223439227d5d7d5d7d"

        let shorterMessage = String(crashMessage.dropFirst(2))
        let longerMessage = "6161\(crashMessage)"
        let alsoCrashMessage = "61\(crashMessage)"

        var data = Data(hex: shorterMessage)!
        var plaintextBuffer = channel.allocator.buffer(capacity: 512)
        plaintextBuffer.write(bytes: data)
        handler.write(ctx: context, data: NIOAny(plaintextBuffer), promise: nil)

        print("Passed for \(data.count) bytes\n")

        data = Data(hex: longerMessage)!
        plaintextBuffer = channel.allocator.buffer(capacity: 512)
        plaintextBuffer.write(bytes: data)
        handler.write(ctx: context, data: NIOAny(plaintextBuffer), promise: nil)

        print("Passed for \(data.count) bytes\n")

        data = Data(hex: crashMessage)!
        plaintextBuffer = channel.allocator.buffer(capacity: 512)
        plaintextBuffer.write(bytes: data)
        handler.write(ctx: context, data: NIOAny(plaintextBuffer), promise: nil)

        print("Passed for \(data.count) bytes\n")

        data = Data(hex: alsoCrashMessage)!
        plaintextBuffer = channel.allocator.buffer(capacity: 512)
        plaintextBuffer.write(bytes: data)
        handler.write(ctx: context, data: NIOAny(plaintextBuffer), promise: nil)

        print("Passed for \(data.count) bytes\n")

    }

The last two tests cause a precondition failure in ByteBuffer.moveWriterIndex

@gbrooker
Copy link
Contributor Author

gbrooker commented Apr 2, 2019

Output of the above test:

Test Suite 'Selected tests' started at 2019-04-03 00:19:43.016
Test Suite 'HAPTests.xctest' started at 2019-04-03 00:19:43.017
Test Suite 'CryptographerTests' started at 2019-04-03 00:19:43.017
Test Case '-[HAPTests.CryptographerTests testCrash]' started.
[hap.encryption|DEBUG] Shared key: 0000000000000000000000000000000000000000000000000000000000000000
[hap.encryption|DEBUG] Decrypt key: cdfb01cb7dc4e38a05503200ffb9c67cc4ad4904b6bf.......
[hap.encryption|DEBUG] Encrypt key: 581cf63d183319a7a0bacc420a250641567d9772488c850......
[hap.encryption|INFO] Encrypt message #0, length: 494
Passed for 494 bytes

[hap.encryption|INFO] Encrypt message #1, length: 497
Passed for 497 bytes

[hap.encryption|INFO] Encrypt message #2, length: 495
Precondition failed: new writerIndex: 513, expected: range(0, 512): file /Users/guy/Documents/macOS-Dev/HomeKit/HAP/.build/checkouts/swift-nio.git--6096836136898517704/Sources/NIO/ByteBuffer-core.swift, line 795
2019-04-03 00:19:43.118286+0800 xctest[35480:2347171] Precondition failed: new writerIndex: 513, expected: range(0, 512): file /Users/guy/Documents/macOS-Dev/HomeKit/HAP/.build/checkouts/swift-nio.git--6096836136898517704/Sources/NIO/ByteBuffer-core.swift, line 795
Program ended with exit code: 9

@Bouke
Copy link
Owner

Bouke commented Apr 2, 2019

Ah yes, this was caused by the buffer not having the right capacity. On entering that method, 2 bytes already had been written; so whenever the requested capacity would be BUCKET_SIZE or BUCKET_SIZE-1, it would result in an buffer overflow. This should be fixed in master in 0cda018.

@gbrooker
Copy link
Contributor Author

gbrooker commented Apr 3, 2019

Excellent, fix works perfectly.

@gbrooker gbrooker closed this as completed Apr 3, 2019
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

2 participants