Added verifying of encoded RLP length to be not greater than available data size #1073
Conversation
…e object size + test
There was a problem hiding this comment.
I'd also add that check to this method: https://github.com/ethereum/ethereumj/pull/1073/files#diff-424bd3538481cd5c9df18504819120b6R776
| private static void verifyLength(int suppliedLength, int availableLength) { | ||
| if (suppliedLength > availableLength) { | ||
| throw new RuntimeException(String.format("Length parsed from RLP (%s bytes) is greater " + | ||
| "than remaining size of data (%s bytes)", suppliedLength, availableLength)); |
There was a problem hiding this comment.
I think it'd be better to read something like than [probable | possible] size of data
|
Addressing my comment above. It's better to add the check to https://github.com/ethereum/ethereumj/pull/1073/files#diff-424bd3538481cd5c9df18504819120b6R801 method where decoding is happened for two reasons: i) it's more convenient for users of |
this one is encoding |
Inpspired by ethereum/ethereumj#1073 Cannot really use the exact test-case there as it is lgpl and we are MIT But the input here is basically doing the same.
Verifying of decoded RLP object length to be not greater than data size
While primary decode method,
RLP.decode2(byte[])is protected to verify whether decoded length lies in the boundaries of remaining data length, we have several other public methods which doesn't include such inspection. One of them,RLP.decode(byte[], int)is used, for example, inValueconstructor and could be attacked using incorrect RLP data. Length is int, so maximum possible size is 2+B or more than 2Gb occupied by each RLPItem from attacker's RLP. So this kind of attack could occupy all available memory.What's done: