Variable-length cells can lie about their length
I'm not sure if this is actually a bug but reporting anyway just to be safe.
Reading through the cell parsing code, I did not see any checks to ensure a variable-length cell is actually as long as it claims to be. In
buffers.c, the only check performed is that enough data exists to read from the buffer, and then the data is just copied into a variable-length cell.
Apologies if I misunderstand tor's code, but it seems to be that:
- Multiple different circuits (from potentially different origins) can share the same connection to another relay, thus
- A connection buffer can hold cells from multiple circuits from different endpoints.
Therefore, if my reading of the code is correct, relay A could send a variable-length cell to relay B claiming to be of length X, but actually only sending Y bytes of data, Y < X, then continuing to send cells normally. This would cause relay B to read X bytes from its connection buffer, corrupting the rest of the cells on that connection buffer.
The end result of corrupting the cells on this connection buffer seems like it would be either:
- (most likely) Most of the rest of the cells on the buffer appear to have "unknown commands" (e.g. garbage in the header) and just get dropped, or
- A cell as a valid command but now appears out-of-order from a client/exit standpoint, so the end-to-end digests will no longer match
Additionally, since relay A gets to choose how many bytes from the connection buffer should be read, relay A can (probably) decide which of these cases are hit (e.g. claim payload length of 513/515 and only send 1 byte to cause relay B to effectively drop the next relay cell, or claim some other arbitrary value to likely corrupt future reads on that connection and probably drop remaining cells in the buffer).
I think the end result would just be the connection getting torn down. Yawning pointed out on irc that, in practice, this might basically be equivalent to relay A just dropping/corrupting cells itself.
I haven't thought of any practical situations where this would cause real problems that differ from relay A just doing bad things to cells directly. It just seems troublesome because it can cause the next relay on the path to silently drop cells and/or tear down the connection, and I'm wondering if there might be scenarios in which relay A can make it appear to a client/other relay that it's actually relay B doing bad things, when it is in fact relay A sending malformed/incomplete variable-length cells.
It's not clear to me how it could even be checked that a variable-length cell is actually the length it claims to be, though.
Sorry if this isn't actually an issue and, as Yawning pointed out, maybe the answer is just "fail the way tor currently fails". I wanted to see what people thought just to be sure.