concordium-node icon indicating copy to clipboard operation
concordium-node copied to clipboard

Use a verifier instead of catch_unwind when deserializing network messages.

Open abizjak opened this issue 3 years ago • 2 comments

Task description

Use a flatbuffers verifier instead of relying on catch_unwind.

Previously this was not easily possible since verifiers were not exposed, but version 2.0 of the flatbuffers dependency does expose a verifier, which we should use.

The verifier should check that internal offsets are valid and do not lead to out-of-bounds errors or other problematic behaviour.

abizjak avatar Jul 06 '21 09:07 abizjak

According to my investigation, the auto-generated code created by the recent flatc (v2.0.6) already contains the verification logic. But it seems to be still experimental as described by the following quote from the auto-generated code:

/// Verifies that a buffer of bytes contains a size prefixed
/// `NetworkMessage` and returns it.
/// Note that verification is still experimental and may not
/// catch every error, or be maximally performant. For the
/// previous, unchecked, behavior use
/// `size_prefixed_root_as_network_message_unchecked`.
pub fn size_prefixed_root_as_network_message(buf: &[u8]) -> Result<NetworkMessage, flatbuffers::InvalidFlatbuffer> {

So, it seems to be reasonable to keep the catch_unwind logic for a while more until a stable implementation arrives.

rimbi avatar Apr 22 '22 06:04 rimbi

Waiting for flatbuffer implementation including this.

smh1001 avatar May 05 '22 12:05 smh1001