# #37462 \[BC-Low] Invalid RLP decoding for single bytes **Submitted on Dec 5th 2024 at 10:45:39 UTC by @Franfran for** [**Attackathon | Ethereum Protocol**](https://immunefi.com/audit-competition/ethereum-protocol-attackathon) * **Report ID:** #37462 * **Report Type:** Blockchain/DLT * **Report severity:** Low * **Target:** * **Impacts:** * (Specifications) A bug in specifications with no direct impact on client implementations ## Description ## Brief/Intro The `decodeOne` function in the `RLP` class has a bug which makes it accept invalid inputs for an encoded value that has the same prefix as a single byte and could trigger consensus failures if the MPT contains some invalid state. ## Vulnerability Details There is a corectness bug in the implementation of the `RLP.decodeOne` function.\ Indeed, if we look at the [Ethereum official documentation about the RLP format](https://ethereum.org/en/developers/docs/data-structures-and-encoding/rlp/), it is stated that ``` RLP encoding is defined as follows: ... - For a single byte whose value is in the `[0x00, 0x7f]` (decimal `[0, 127]`) range, that byte is its own RLP encoding. ... ``` By looking at the `decodeOne` function code, we can see the case of the single byte as: ```java public static Bytes decodeOne(final Bytes encodedValue) { if (encodedValue.size() == 0) { throw new RLPException("Invalid empty input for RLP decoding"); } final int prefix = encodedValue.get(0) & 0xFF; final RLPDecodingHelpers.Kind kind = RLPDecodingHelpers.Kind.of(prefix); if (kind.isList()) { throw new RLPException(format("Invalid input: value %s is an RLP list", encodedValue)); } if (kind == RLPDecodingHelpers.Kind.BYTE_ELEMENT) { return encodedValue; } /* ... */ ``` We can see that if the prefix is a single byte, the full **encodedValue** is returned while it is not checked that this value is indeed a single byte.\ Let's write a test to confirm this behavior, which can be done by extending `RLPTest.java` ## Impact Details This function is called twice in the codebase. 1. 2. The only relevant one is 2. because in the case of 1., only a single byte can be passed to the `decodeOne` function. In the case of 2., if an invalid RLP value would be stored in the Merkle Patricia Trie (MPT) that would wrongly be decoded by this function such that it starts with a `SINGLE_BYTE` and ends up with other bytes, while other clients with the correct implementation would disagree with the world view of Besu, triggering a consensus failure. Because of this assumption, the chosen vulnerability level was set to `LOW`. Nonetheless, it is important to consider this report in order to fix the implementation since future code may rely on the assumption that any RLP-encoded value whose prefix is the single byte will error like other implementations.\ Let's for instance consider `go-ethereum`, which has a different implementation of the decoding of an RLP value whose prefix is a single byte: ```go func (s *Stream) readKind() (kind Kind, size uint64, err error) { b, err := s.readByte() /* ... */ s.byteval = 0 switch { case b < 0x80: // For a single byte whose value is in the [0x00, 0x7F] range, that byte // is its own RLP encoding. s.byteval = b return Byte, 0, nil /* ... */ ``` Then, the prefix is of type `Byte` and is validated correctly: ```go func decodeByteArray(s *Stream, val reflect.Value) error {••••••••••••• kind, size, err := s.Kind() if err != nil { return err } slice := byteArrayBytes(val, val.Len()) switch kind { case Byte: if len(slice) == 0 { return &decodeError{msg: "input string too long", typ: val.Type()} } else if len(slice) > 1 { return &decodeError{msg: "input string too short", typ: val.Type()} } slice[0] = s.byteval s.kind = -1 ``` Since it's going to return a `decodeError` if the length of the slice is not 1. ## References ## Proof of Concept ## Proof of Concept ```java @Test public void decodeOneSuffixBytes() { Assertions.assertThrows(RLPException.class, () -> RLP.decodeOne(Bytes.fromHexString("0x7f1234"))); // this is invalid RLP } ``` As expected, this test fails because the function doesn't throw as it should.