square / square/wire

Malformed packed varints read beyond field length

Open
#3,050 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Kotlin
Stars
4.4k
Forks
627
Avg merge
3d 15m
Merged PRs (30d)
20

Description

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.)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the Wire-Swift and Kotlin protobuf decoding implementations and reproduce the malformed packed-varint input from the issue. Compare their handling with the linked protobuf conformance tests, including truncated nested messages; done means malformed input is rejected without consuming bytes from the following fields.

Written by the indexing model from the issue text.

Assessment

Tech stack
kotlin, swift
Domain
backend-api-design
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.