4 ms·

that's not really a "flaw" in protobuf; the protobuf is Working As Intended by decoding the field in a different place when you modify it. i am not sure how on

that's not really a "flaw" in protobuf; the protobuf is Working As Intended by decoding the field in a different place when you modify it. i am not sure how one could possibly characterize that as a "flaw," since protobuf is a field-numeric length-prefixed protocol in the first place. it makes a (reasonable) assumption that bytes won't be messed with over the wire, leaving integrity to the reader, so even if this _was_ a flaw, it would be a flaw in the YouTube app for iOS, not in protobuf... since it isn't a flaw, it isn't an "exploit," unless you are referring to the fact that youtube's protobuf exchange on their iOS app isn't checking hashes for returned payloads. i suspect after this post they will

playingalong 3y ago
It says January 2022. So likely if they wanted to harden the protocol after the blog post, they would already have.
- can16358p 3y ago
  But perhaps the article has just got popular now, due to HN?
- sgammon 3y ago
  Thanks for pointing that out, I hadn't noticed that.
newaccount74 3y ago
Yeah, that's some weird editorializing on behalf of the author. The "flaw" is mentioned only in the headline, the text just explains how the format works. It's not a flaw, just working as designed. There's also this bit: > Google makes it computationally expensive to decode, alter, and re-encode without the C++ source proto files Yeah, it's computationally expensive if you use unoptimized Python code to do it. If you write your code in C (or another compiled language) then scanning 1.8MB of protobuf code should be trivial, with or without the proto source files. I'm pretty sure that making Protobuf files hard to decode without sources is not a design goal. If it was, they did a pretty lousy job.
- kccqzy 3y ago
  Exactly. It's not exactly computationally expensive if you can do it at 2GB/s! https://news.ycombinator.com/item?id=26931581 https://news.ycombinator.com/item?id=26931581
  - fsckboy 3y ago
    it's computationally expensive if it's more computation than all the other things you'd like your hardware to do, i think is a better definition.
    - newaccount74 3y ago
      No, that's not the issue. The author seemed to have parsed the protobuf data by converting it all to a huge hierarchy of thousands (millions?) of Python objects. Of course that is slow and takes a few seconds. That's so many allocations! But if you are just looking for a specific tag, there's no need for that, I'm pretty sure you can parse protobuf in place without allocations, which would be a million times faster. You could maybe even do it in Python with reasonable speed.
      - MayeulC 3y ago
        Heck, you may be able to do it with eBPF or some other low-level hardware-accelerated packet filter.
layer8 3y ago
I don’t know how mandatory fields work in Protobuf, but to mitigate the attack, Google’s YouTube client could treat the field as mandatory and refuse service when it finds the field to be missing or having a default value.
- sgammon 3y ago
  Protocol buffers don't always support "default" values, because certain types will include a protocol-default value. For example, a `bool` which is not present will return `false` instead of say, a nil type. Required fields were a feature of Proto2. In proto3 syntax (latest), the required field concept was dropped because it caused issues with protocol evolution and was easy to misuse. In essence, because of the backward and forward compatibility guarantees supported by protobuf, a "required" field must be required for the entire lifecycle of the protocol. For these reasons and others, protobuf takes a stance where unrecognized fields are not necessarily errors. If it took a strict stance and failed in this condition, the presence of new fields in an "evolved" protocol would be an error which would break forward compatibility; old clients would not be able to communicate with new servers, and vice versa. Protobuf guarantees that new fields will not break forward or backward compatibility. This is why parsing unknown fields in protobuf is a feature, working as intended, not a flaw. In some language SDKs for protobuf I believe you can customize this behavior but it really isn't a good idea. This is also why the app authors might want to consider a hash instead. Tampering with the payload would break the hash, and without the schema, the author would not necessarily know where the hash was situated in the payload to fix it, or even that one is present at all. The complexity of recalculating the hash (assuming they find it) vastly multiplies the attacker's burden at little cost to the application; adding a few rounds and a salt, for instance, would make this kind of attack significantly harder to pull off. It's not perfect security, but it would certainly be better.
  - layer8 3y ago
    Well, for the field in question, I imagine that it should be easy to distinguish meaningful content from whatever it defaults to when the field is missing.

playingalong 3y ago

It says January 2022. So likely if they wanted to harden the protocol after the blog post, they would already have.

can16358p 3y ago
But perhaps the article has just got popular now, due to HN?
sgammon 3y ago
Thanks for pointing that out, I hadn't noticed that.

newaccount74 3y ago

Yeah, that's some weird editorializing on behalf of the author. The "flaw" is mentioned only in the headline, the text just explains how the format works. It's not a flaw, just working as designed. There's also this bit: > Google makes it computationally expensive to decode, alter, and re-encode without the C++ source proto files Yeah, it's computationally expensive if you use unoptimized Python code to do it. If you write your code in C (or another compiled language) then scanning 1.8MB of protobuf code should be trivial, with or without the proto source files. I'm pretty sure that making Protobuf files hard to decode without sources is not a design goal. If it was, they did a pretty lousy job.

kccqzy 3y ago
Exactly. It's not exactly computationally expensive if you can do it at 2GB/s! https://news.ycombinator.com/item?id=26931581 https://news.ycombinator.com/item?id=26931581
- fsckboy 3y ago
  it's computationally expensive if it's more computation than all the other things you'd like your hardware to do, i think is a better definition.
  - newaccount74 3y ago
    No, that's not the issue. The author seemed to have parsed the protobuf data by converting it all to a huge hierarchy of thousands (millions?) of Python objects. Of course that is slow and takes a few seconds. That's so many allocations! But if you are just looking for a specific tag, there's no need for that, I'm pretty sure you can parse protobuf in place without allocations, which would be a million times faster. You could maybe even do it in Python with reasonable speed.
    - MayeulC 3y ago
      Heck, you may be able to do it with eBPF or some other low-level hardware-accelerated packet filter.

layer8 3y ago

I don’t know how mandatory fields work in Protobuf, but to mitigate the attack, Google’s YouTube client could treat the field as mandatory and refuse service when it finds the field to be missing or having a default value.

sgammon 3y ago
Protocol buffers don't always support "default" values, because certain types will include a protocol-default value. For example, a `bool` which is not present will return `false` instead of say, a nil type. Required fields were a feature of Proto2. In proto3 syntax (latest), the required field concept was dropped because it caused issues with protocol evolution and was easy to misuse. In essence, because of the backward and forward compatibility guarantees supported by protobuf, a "required" field must be required for the entire lifecycle of the protocol. For these reasons and others, protobuf takes a stance where unrecognized fields are not necessarily errors. If it took a strict stance and failed in this condition, the presence of new fields in an "evolved" protocol would be an error which would break forward compatibility; old clients would not be able to communicate with new servers, and vice versa. Protobuf guarantees that new fields will not break forward or backward compatibility. This is why parsing unknown fields in protobuf is a feature, working as intended, not a flaw. In some language SDKs for protobuf I believe you can customize this behavior but it really isn't a good idea. This is also why the app authors might want to consider a hash instead. Tampering with the payload would break the hash, and without the schema, the author would not necessarily know where the hash was situated in the payload to fix it, or even that one is present at all. The complexity of recalculating the hash (assuming they find it) vastly multiplies the attacker's burden at little cost to the application; adding a few rounds and a salt, for instance, would make this kind of attack significantly harder to pull off. It's not perfect security, but it would certainly be better.
- layer8 3y ago
  Well, for the field in question, I imagine that it should be easy to distinguish meaningful content from whatever it defaults to when the field is missing.