Malformed packed varints read beyond field length

[jrose-signal]

With the following protobuf

syntax = "proto3";

message RepeatedBug {
  bytes name = 1;
  repeated int32 values = 2;
  int32 id = 3;
  bytes should_never_appear = 4;
}

And the following (malformed) input:

12       // (tag 2 | Length Delimited)
01       // Length of 'values'
80       // *** Invalid truncated varint ***
18       // (tag 3 | Varint)
22       // ID value 34, which happens to be equivalent to (tag 4 | Length Delimited)
0A       // (tag 1 | Length Delimited), which happens to be equivalent to a length of 10
09       // Length of 'name'
313233   // UTF-8 Value "123456789"
343536
373839

(which can be generated by this convenient protoscope):

2:LEN {`80`}
3:VARINT 4:LEN
1:LEN {"123456789"}

Wire-Swift treats the 3:VARINT as part of the length-delimited field, and produces RepeatedBug(values: [3072], should_never_appear: "\u{09}123456789"), instead of rejecting the input as malformed. (Most instances of this bug would result in an unexpected error somewhere else, as the input stream has been desynchronized, but I managed to find this one that puns to a different "valid" structure instead.)

As far as I can tell the Kotlin implementation has the same bug, but I didn't test it. Similarly, both implementations seem vulnerable to the same issue when it's a nested message that's truncated, rather than a packed field. (It would not surprise me if these were covered by Google's protobuf conformance tests, but I didn't check that either.)

[jrose-signal]

Here's the test I added for Swift (in addition to the above proto):

func testDecodePackedRepeatedVarintMalformed() throws {
    // Protoscope input:
    // 2:LEN {`80`}
    // 3: 4:LEN
    // 1:LEN {"123456789"}
    let data = Foundation.Data(hexEncoded: """
        12       // (tag 2 | Length Delimited)
        01       // Length of 'values'
        80       // Invalid truncated varint
        18       // (tag 3 | Varint)
        22       // ID value 34, which happens to be equivalent to (tag 4 | Length Delimited)
        0A       // (tag 1 | Length Delimited), which happens to be equivalent to a length of 10
        09       // Length of 'name'
        313233   // UTF-8 Value "123456789"
        343536
        373839
    """)!
    try test(data: data) { reader in
        do {
            let result = try reader.decode(RepeatedBug.self)
            XCTFail("should have failed to parse, but got \(result)")
        } catch is ProtoDecoder.Error {
            // Okay, we got the expected error.
        }
    }
}

[oldergod]

Thanks for reporting. I didn't work much on the Swift side so I cannot really help here. I've checked what Kotlin does and we fail like so:

java.io.IOException: Expected to end at 3 but was 4
    at com.squareup.wire.ProtoReader.afterPackableScalar(ProtoReader.kt:433)
    at com.squareup.wire.ProtoReader.readVarint32(ProtoReader.kt:325)
    at com.squareup.wire.ProtoAdapterKt$commonInt32$1.decode(ProtoAdapter.kt:906)
    at com.squareup.wire.ProtoAdapterKt$commonInt32$1.decode(ProtoAdapter.kt:886)

And protoc failed as follows:

While parsing a protocol message, the input ended unexpectedly in the middle of a field.  This could mean either that the input has been truncated or that an embedded message misreported its own length.
com.google.protobuf.InvalidProtocolBufferException: While parsing a protocol message, the input ended unexpectedly in the middle of a field.  This could mean either that the input has been truncated or that an embedded message misreported its own length.
    at com.google.protobuf.InvalidProtocolBufferException.truncatedMessage(InvalidProtocolBufferException.java:92)
    at com.google.protobuf.CodedInputStream$ArrayDecoder.readRawByte(CodedInputStream.java:1217)
    at com.google.protobuf.CodedInputStream$ArrayDecoder.readRawVarint64SlowPath(CodedInputStream.java:1102)
    at com.google.protobuf.CodedInputStream$ArrayDecoder.readRawVarint32(CodedInputStream.java:996)
    at com.google.protobuf.CodedInputStream$ArrayDecoder.readInt32(CodedInputStream.java:747)

If you feel like fixing the Swift adapter, please do! Otherwise, we'll try to fix it later.