Crash in ChaCha20Poly1305.encrypt

[gbrooker]

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]

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]

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 0cda01852710e3cc70ca985a475b8c323ac6b062.

[gbrooker]

Excellent, fix works perfectly.