Bug Fix: Catch further encoding exception when verifying message #228
Conversation
| try { | ||
| encoded_signing_payload = msg.signing_payload(); | ||
| } catch (...) { | ||
| confirm.report = VerificationReport::False_Signature; |
There was a problem hiding this comment.
Maybe we should add a more descriptive report value, e.g. Corrupt_Message or Encoding_Failure?
There was a problem hiding this comment.
Corrupt_Message sounds good, I can do that. I have seen that in security::DelegatingSecurityEntity::decapsulate_packet:39, VerificationReport from the verify services is cast to DecapReport from SN-Decaps. Is it intentional that Unencrypted_Message and Decryption_Error are not included in VerificationReport? Decryption_Error is maybe another good candidate.
Unfortunately, I haven't found anything about this, but is it perhaps possible with asn1c to extract the indices and length of certain elements in the bytes to decode? That way, we could skip the entire encode step.
There was a problem hiding this comment.
Good catch regarding the report casting. For the sake of robustness, this cast should be replaced by a proper mapping function.
I am not aware of any smart way to get the length of asn1c data structures without actually calling the encoder. You can call the encoding stage with a "null sink" though, i.e. no actual bytes are written only the "written bytes" are tracked.
There was a problem hiding this comment.
I have annotated the enums with values according to AUTOSAR SWS_V2xM_91000 so that the cast at least remains consistent. I hope it's okay that I added the None enum according to the AUTOSAR specification.
When fuzzing with additional seeds, I noticed that
vanetza::security::v3::SecuredMessage::signing_payloadcan throw an exception. For this reason, I assume thatvanetza::asn1::encode_oershould never be called without proper exception handling. I have added further exception handlings in this pull request.