flow-go icon indicating copy to clipboard operation
flow-go copied to clipboard

Use more restrictive decode mode in global CBOR Decoder

Open jordanschalm opened this issue 11 months ago • 1 comments

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: https://github.com/onflow/flow-go/blob/e8cec7a33bc934a345827fcb69f2f1f6d88dc545/model/encoding/cbor/codec.go#L1
  • See discussion here: https://github.com/onflow/flow-go/pull/5428#discussion_r1511987247
  • See also this enumeration of where different encoders are used in flow-go: https://github.com/onflow/flow-go/issues/5305#issuecomment-1917635711

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
  • Add test case -- decoding a message with extra fields should return an error

jordanschalm avatar Mar 11 '24 16:03 jordanschalm