Protobuf string decoding in adapter google/googledatatransport-firelog-batchlog-protobuf

[baltpeter]

In google/googledatatransport-firelog-batchlog-protobuf (which I'm writing as part of #52), I'm seeing some odd values:

That is not us misinterpreting a field or otherwise specifying a wrong dataPath, it's a problem of our Protobuf decoder (which we took from Cyberchef).

Here's an example of a request where that happens: https://data.tweasel.org/data/requests/informed-consent,101672

If we load that in Cyberchef, we get the same problem:

The fields I marked with an arrow should be iPhone9,3 and en-GB, respectively but they are instead interpreted as buffers.

As far as I understand it, there is no definitive way to tell from the Protobuf wireformat (without a schema) whether a value is a string or buffer, so libraries apply heuristics, which can of course fail.

https://protogen.marcgravell.com/decode handles this case correctly/offers both interpretations:

I also quickly tried rawprotoparse (which is used by HTTPToolkit), but even with rawprotoparse(buffer, { stringMode: 'string' }), that still interprets both fields as buffers:

[baltpeter]

To make debugging this easier, I created an MRE using https://www.protobufpal.com/.

---

Schema:

syntax = "proto3";

message Message {
  string parsedIncorrectly = 5;
  string parsedCorrectly = 7;
}

Decoded message JSON:

{
  "parsedIncorrectly": "iPhone9,3",
  "parsedCorrectly": "com.chargemap"
}

Encoded message base64:

KglpUGhvbmU5LDM6DWNvbS5jaGFyZ2VtYXA=

---

This exhibits the exact same problem.

Without a schema, Cyberchef parses it as:

{
    "5": {
        "13": 3687385302716868600
    },
    "7": "com.chargemap"
}

With the schema, it is parsed correctly:

{
    "parsedIncorrectly": "iPhone9,3",
    "parsedCorrectly": "com.chargemap"
}

[baltpeter]

This is clearly some heuristic that is failing. While iPhone9,3 is parsed incorrectly, bPhone9,3 (KgliUGhvbmU5LDM6DWNvbS5jaGFyZ2VtYXA=) is parsed correctly, for example.

[baltpeter]

Unlike on the full protobuf from the request, Cyberchef's "Show Types" option does work for our MRE:

{
    "field #5: L-delim (e.g. string, message)": {
        "field #13: 64-Bit (e.g. fixed64, double)": 3687385302716868600
    },
    "field #7: L-delim (e.g. string, message)": "com.chargemap"
}

[baltpeter]

This is the relevant function:

TrackHAR/src/common/protobuf.mjs

Lines 590 to 609 in 36d2292

	_lenDelim(fieldNum) {
	// Read off the field length
	const length = this._varInt();
	const fieldBytes = this.data.slice(this.offset, this.offset + length);
	let field;
	try {
	// Attempt to parse as a new Protobuf Object
	const pbObject = new Protobuf(fieldBytes);
	field = pbObject._parse();
	// Set field types object
	this.fieldTypes[fieldNum] = { ...this.fieldTypes[fieldNum], ...pbObject.fieldTypes };
	} catch (err) {
	// Otherwise treat as bytes
	field = byteArrayToChars(fieldBytes);
	}
	// Move the offset and return the field
	this.offset += length;
	return field;
	}

Both fields are length-delimited fields. For those, the code tries to recursively parse them as a Protobuf message. If that fails, it is interpreted as raw bytes/string.

And the way the string iPhone9,3 is encoded in a Protobuf just so happens to be a valid Protobuf message. :/ Really not sure whether there's anything we'll be able to do here.