ChainSafe / ChainSafe/lodestar
Trailing Bytes Spec Violation
- Dominant language
- TypeScript
- Stars
- 1.4k
- Forks
- 483
- Avg merge
- 1d 16h
- Merged PRs (30d)
- 150
Description
### Describe the bug
## Summary
Hello developers!
We are conducting differential testing on CL clients. In our testing, we found that Lighthouse, Prysm, Lodestar, Nimbus, and Grandine accept `Ping` requests containing trailing garbage bytes after the valid SSZ-snappy payload, while the spec requires rejection with `INVALID_REQUEST`.
**Spec reference** [(Phase0 `p2p-interface.md`)](https://github.com/ethereum/consensus-specs/blob/bda4120b35152162984131a0e4f24cab508f4da5/specs/phase0/p2p-interface.md?plain=1#L1294-L1306):
> A reader MUST consider the following cases as invalid input:
> - Any remaining bytes, after having read the `n` SSZ bytes.
>
> In case of an invalid input (header or payload), a reader MUST:
> - From requests: send back an error message, response code `InvalidRequest`.
## What happens
```
Wire: [varint(8)] [snappy(uint64_le(1))] [0xDE 0xAD ... 16 garbage bytes]
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Spec says: MUST reject as invalid
```
Lighthouse, Prysm, Lodestar, Nimbus, and Grandine read `n` bytes declared in the varint header, decode the Ping payload, return `SUCCESS`, then close the stream. But the spec requires to treat this as `InvalidRequest`.
## Results
| Client | Response | Spec Compliant |
| ---------- | --------------- | :------------: |
| Teku | INVALID_REQUEST | YES |
| Grandine | SUCCESS | NO |
| Lighthouse | SUCCESS | NO |
| Lodestar | SUCCESS | NO |
| Nimbus | SUCCESS | NO |
| Prysm | SUCCESS | NO |
Thanks for your attention!
### Expected behavior
Reject with `InvalidRequest` response code.
### Steps to reproduce
Please check this [link](https://gist.github.com/Alleysira/1075ff4d5623c834468b2c2945bd623d) to reproduce with kurtosis.
### Additional context
_No response_
### Operating system
Linux
### Lodestar version or commit hash
chainsafe/lodestar:v1.42.0
Contributor guide
Assessment
This issue has not been assessed yet.