Open
Description
Context
The CBOR library provides an option to return an error while decoding if the input has a field which does not exist in the target Go struct. We should globally enable this option by default in flow-go
as it avoids some surface area for spam which currently exists. In particular, a malicious sender can bloat the size of a message without it being detected:
- use extra bandwidth and memory on the victim
- send semantically equivalent messages, that are considered binary-different by the networking layer, and therefore are not de-duplicated or detected by the networking layer
- the attacker would not be able to bloat the size of persistent objects (like blocks etc.) because all recipients will decode then re-encode before persisting to disk or propagating to other nodes
- Here is the relevant decoding option: https://github.com/fxamacker/cbor/blob/master/decode.go#L513
- Here is where the global cbor decoder is defined:
flow-go/model/encoding/cbor/codec.go
Line 1 in e8cec7a
- See discussion here: [KV Store]
ProtocolStateVersionUpgrade
Service Event #5428 (comment) - See also this enumeration of where different encoders are used in flow-go: [Dynamic Protocol State | M1] KV Store core types & serialization #5305 (comment)
Definition of Done
- Change the global decode options
- Validate:
- the codebase's use of CBOR should use the global decode options (should not directly call
cbor
library, bypassing decode options specification) - all existing models and patterns are compatible with this stricter decoding mode
- the codebase's use of CBOR should use the global decode options (should not directly call
- Add test case -- decoding a message with extra fields should return an error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment