# #38828 \[BC-Low] Decode RLP of Legacy Transaction Allows Tailing Bytes **Submitted on Jan 14th 2025 at 19:33:55 UTC by @CertiK for** [**Attackathon | Ethereum Protocol**](https://immunefi.com/audit-competition/ethereum-protocol-attackathon) * **Report ID:** #38828 * **Report Type:** Blockchain/DLT * **Report severity:** Low * **Target:** * **Impacts:** * (Specifications) A bug in specifications with no direct impact on client implementations ## Description ## Brief/Intro RLP serialization is widely used in the Ethereum clients, which provides a standard format for the data transfer between nodes. In Erigon ( ), the decode RLP of legacy tx misses check the tailing bytes, which allows extra tailing bytes during the RLP decoding. This could possibly be a consensus issue as other Ethereum clients reject such legacy transactions. ## Vulnerability Details Affected Codebase:\ The function `DecodeRLPTransaction()` is intended to decode the RLP encoding of transactions: ``` func DecodeRLPTransaction(s *rlp.Stream, blobTxnsAreWrappedWithBlobs bool) (Transaction, error) { kind, _, err := s.Kind() if err != nil { return nil, err } if rlp.List == kind { tx := &LegacyTx{} if err = tx.DecodeRLP(s); err != nil { return nil, err } return tx, nil } if rlp.String != kind { return nil, fmt.Errorf("not an RLP encoded transaction. If this is a canonical encoded transaction, use UnmarshalTransactionFromBinary instead. Got %v for kind, expected String", kind) } // Decode the EIP-2718 typed TX envelope. var b []byte if b, err = s.Bytes(); err != nil { return nil, err } if len(b) == 0 { return nil, rlp.EOL } return UnmarshalTransactionFromBinary(b, blobTxnsAreWrappedWithBlobs) } ``` For non-legacy transactions, it invokes `UnmarshalTransactionFromBinary()`that handles the tailing bytes with the `check s.Remaining() != 0`: ``` func UnmarshalTransactionFromBinary(data []byte, blobTxnsAreWrappedWithBlobs bool) (Transaction, error) { if len(data) <= 1 { return nil, fmt.Errorf("short input: %v", len(data)) } s := rlp.NewStream(bytes.NewReader(data[1:]), uint64(len(data)-1)) var t Transaction switch data[0] { case AccessListTxType: t = &AccessListTx{} case DynamicFeeTxType: t = &DynamicFeeTransaction{} case BlobTxType: if blobTxnsAreWrappedWithBlobs { t = &BlobTxWrapper{} } else { t = &BlobTx{} } case SetCodeTxType: t = &SetCodeTransaction{} default: if data[0] >= 0x80 { // Tx is type legacy which is RLP encoded return DecodeTransaction(data) } return nil, ErrTxTypeNotSupported } if err := t.DecodeRLP(s); err != nil { return nil, err } if s.Remaining() != 0 { return nil, fmt.Errorf("trailing bytes after rlp encoded transaction") } return t, nil } ``` However, if it’s the legacy transaction, it calls `tx.DecodeRLP(s)` to directly decode the legacy transaction, in which the tailing bytes is unchecked. This oversight would allow tailing bytes to sneak into the legacy transaction when calling `DecodeRLPTransaction()` to decode the legacy transaction. ``` func (tx *LegacyTx) DecodeRLP(s *rlp.Stream) error { _, err := s.List() if err != nil { return fmt.Errorf("legacy tx must be a list: %w", err) } if tx.Nonce, err = s.Uint(); err != nil { return fmt.Errorf("read Nonce: %w", err) } var b []byte if b, err = s.Uint256Bytes(); err != nil { return fmt.Errorf("read GasPrice: %w", err) } tx.GasPrice = new(uint256.Int).SetBytes(b) if tx.Gas, err = s.Uint(); err != nil { return fmt.Errorf("read Gas: %w", err) } if b, err = s.Bytes(); err != nil { return fmt.Errorf("read To: %w", err) } if len(b) > 0 && len(b) != 20 { return fmt.Errorf("wrong size for To: %d", len(b)) } if len(b) > 0 { tx.To = &libcommon.Address{} copy((*tx.To)[:], b) } if b, err = s.Uint256Bytes(); err != nil { return fmt.Errorf("read Value: %w", err) } tx.Value = new(uint256.Int).SetBytes(b) if tx.Data, err = s.Bytes(); err != nil { return fmt.Errorf("read Data: %w", err) } if b, err = s.Uint256Bytes(); err != nil { return fmt.Errorf("read V: %w", err) } tx.V.SetBytes(b) if b, err = s.Uint256Bytes(); err != nil { return fmt.Errorf("read R: %w", err) } tx.R.SetBytes(b) if b, err = s.Uint256Bytes(); err != nil { return fmt.Errorf("read S: %w", err) } tx.S.SetBytes(b) if err = s.ListEnd(); err != nil { return fmt.Errorf("close tx struct: %w", err) } return nil } ``` ## Impact Details The function `DecodeRLPTransaction()` has been widely utilized in the Erigon codebase to decode the transactions from block, block body and user’s submitted raw transaction. Since tailing bytes of a transaction is disallowed in other Ethereum clients, it could introduce consensus issues with this flawed decoding function. ## References * ## Proof of Concept ## Proof of Concept For simplicity, we provide the following unit test to compare two transaction decoding functions, `DecodeRLPTransaction()` vs `UnmarshalTransactionFromBinary()`. The encoded legacy transaction is `[]byte{248, 99, 128, 2, 1, 148, 9, 94, 123, 174, 166, 166, 199, 196, 194, 223, 235, 151, 126, 250, 195, 38, 175, 85, 45, 135, 128, 134, 97, 98, 99, 100, 101, 102, 37, 160, 142, 183, 96, 239, 234, 152, 124, 9, 98, 209, 245, 242, 175, 209, 122, 221, 51, 54, 23, 237, 233, 142, 83, 7, 50, 17, 119, 99, 15, 34, 57, 125, 160, 21, 141, 44, 195, 195, 220, 247, 64, 91, 79, 6, 37, 93, 226, 96, 69, 227, 240, 8, 168, 169, 112, 118, 124, 14, 250, 73, 19, 190, 7, 104, 57, 1}` that contains the last taling bytes `0x01`. 1. Run the following unit test: ``` func TestDecodeRLPTransactionTailing(t *testing.T) { input := []byte{248, 99, 128, 2, 1, 148, 9, 94, 123, 174, 166, 166, 199, 196, 194, 223, 235, 151, 126, 250, 195, 38, 175, 85, 45, 135, 128, 134, 97, 98, 99, 100, 101, 102, 37, 160, 142, 183, 96, 239, 234, 152, 124, 9, 98, 209, 245, 242, 175, 209, 122, 221, 51, 54, 23, 237, 233, 142, 83, 7, 50, 17, 119, 99, 15, 34, 57, 125, 160, 21, 141, 44, 195, 195, 220, 247, 64, 91, 79, 6, 37, 93, 226, 96, 69, 227, 240, 8, 168, 169, 112, 118, 124, 14, 250, 73, 19, 190, 7, 104, 57, 1} tx, err := UnmarshalTransactionFromBinary(input, false) if err != nil { fmt.Printf("Unmarshal Tx with UnmarshalTransactionFromBinary() fails: %+v\n", err) } else { fmt.Printf("Unmarshal Tx with UnmarshalTransactionFromBinary() succeeds: %+v\n", tx.Hash()) } data := input s := rlp.NewStream(bytes.NewReader(data), uint64(len(data))) legacyTx, err := DecodeRLPTransaction(s, false) if err != nil { fmt.Printf("Decode RLP of Tx DecodeRLPTransaction() fails: %+v\n", err) } else { fmt.Printf("Decode RLP of Tx DecodeRLPTransaction() succeeds: %+v\n", legacyTx.Hash()) } } ``` 2. The test results show that the decoding of this legacy transaction with tailing bytes succeeds with `DecodeRLPTransaction()` but fails with `UnmarshalTransactionFromBinary()`. ``` === RUN TestDecodeRLPTransactionTailing Unmarshal Tx with UnmarshalTransactionFromBinary() fails: trailing bytes after rlp encoded transaction Decode RLP of Tx DecodeRLPTransaction() succeeds: 0x74487f78cbc2fd511acd00d2d5ec65d9a19fb8eaad38c33b4e5737d780c7cc60 --- PASS: TestDecodeRLPTransactionTailing (0.00s) PASS Process finished with the exit code 0 ``` --- # Agent Instructions: Querying This Documentation If you need additional information that is not directly available in this page, you can query the documentation dynamically by asking a question. Perform an HTTP GET request on the current page URL with the `ask` query parameter: ``` GET https://reports.immunefi.com/ethereum-protocol-or-attackathon/38828-bc-low-decode-rlp-of-legacy-transaction-allows-tailing-bytes.md?ask= ``` The question should be specific, self-contained, and written in natural language. The response will contain a direct answer to the question and relevant excerpts and sources from the documentation. Use this mechanism when the answer is not explicitly present in the current page, you need clarification or additional context, or you want to retrieve related documentation sections.