Safety on the reader side

[Todd Lipcon]

Hey all,

I took a quick look through the code, and didn't see any validation of the embedded offsets inside a serialized flatbuffer object. If I have a "table" instance with embedded pointers to other tables/structs, is there any validation performed on the reader side before accessing these pointers? Without this, it seems pretty dangerous to use this in an untrusted environment.

-Todd

[Wouter van Oortmerssen]

Most readers of binary file formats can be made to misbehave with corrupt data, and FlatBuffers is no different.

FlatBuffers is the lowest level data substrate, systems ensuring data integrity should ideally be built on top of it, rather than inside of it.

That said, it would be possible add an optional mode that bounds-checks all offsets, or maybe better yet, a generated verifier function from the schema.

[Todd Lipcon]

On Tuesday, June 17, 2014 1:26:14 PM UTC-7, Wouter van Oortmerssen wrote:

Most readers of binary file formats can be made to misbehave with corrupt data, and FlatBuffers is no different.

Sure -- for data stored on disk or sent around in trusted systems, I'd build a crc32 or other checksum on top to guard against accidental corruption.

The concern is more about using this as a serialization mechanism for an RPC system where the clients are untrusted from the server's perspective. The flatbuffer web page indicates that it's meant for mobile apps and games, so I'd expect this is the security model in play.

 

FlatBuffers is the lowest level data substrate, systems ensuring data integrity should ideally be built on top of it, rather than inside of it.

That said, it would be possible add an optional mode that bounds-checks all offsets, or maybe better yet, a generated verifier function from the schema.

Yep, seems like a blocker to me before I'd consider using the library in a client-server setting.

 

[Ben Harper]

+1 on the security issue.

There needs to be some way of ensuring that foreign data is not going to be a security vulnerability.

[Bernard van Gastel]

I agree. This would make it useful for network serialization (network play of games), and as a format for external (user generated) game data. This feature does not conflict with the performance requirements as a validator method call can be made optional.

Regards,
Bernard

P.S. Nice work, I like the FlatBuffer approach.

> --
> You received this message because you are subscribed to the Google Groups "FlatBuffers" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to flatbuffers...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[Kai Skye]

First of all, thanks for creating FlatBuffers! It solves a real problem and I know many people who are already interested in it. That said, I think that the issue brought up in this thread is the biggest blocker for me for using this library.

I can build a container format on top of flatbuffers that is suitable for network packets (buffer size, checksum, etc.). And I can verify that the values of each field fall within the constraints set by my application. But there is nothing preventing an attacker from crafting data with a vtable offset and/or vector size that causes flatbuffers to read memory outside of the buffer. If that data happens to fit within the constraints of the field then I might end up sending back important data to the attacker (e.g. session ids of other players).

To expand on what Bernard said, it would be great to have a Verify function that I can pass a number of bytes. The function would iterate over the schema and attempt to access each field. If flatbuffers attempts to access any field whose memory address is greater than the (data's base address) + (number of bytes), then the Verify function would return a failure code and I would know to discard the data. This has the advantage of incurring no additional overhead if I already know that the data can be trusted, since I would just not bothering calling Verify in that case. And I prefer it to having an optional mode that bounds-checks each offset because, (a) that imposes an additional cost on field accesses, and, (b) I wouldn't know if I'm reading only the valid parts of maliciously formed data.

[Wouter van Oortmerssen]

Ok, I see how this would expand the possible use cases of FlatBuffers.

I can generate a verifier function that returns true if it thinks it is impossible to touch memory outside the buffer during a traversal. This means bounds checking all offsets, and checking for string 0-termination.

This will still not stop an attacker from modifying, say, an int field, that you blindly use to index into some array, but that really is the client's responsibility. One thing that can help here is min & max meta data for each field, checking of which can be rolled into this verifier as well.

I'll make this a high priority item.

[Wouter van Oortmerssen]

If you get the latest from hithub, there is now verification functionality, where you can have a buffer verified before you access it. Passing verification means it guarantees that no memory access will end up outside the buffer.