boot: bootutil: bounds-check PSA ECDSA signature parse

[d3zd3z]

parse_signature_from_rfc5480_encoding() walks the DER encoding of an ECDSA
signature using length bytes taken from the buffer itself, without knowing how
many bytes that buffer holds — bootutil_ecdsa_verify() discards its slen
argument outright. The r length at sig[3], the s length derived from it,
and the memcpy source offsets computed from both are all used unchecked, so a
malformed signature causes reads past the end of the caller's signature buffer.

The size cannot be inferred from the encoding either, since the lengths that
drive the walk come from the buffer contents rather than from slen.

This passes the length down and validates the encoding against it before any of
the contents are used:

• reject a buffer too short for the smallest SEQUENCE of two INTEGERs;
• check the SEQUENCE and INTEGER tags the old code only assumed;
• require the outer length and each element length to fit in the bytes that
  remain.

Lengths are compared against the remaining count rather than added to an offset,
so none of the arithmetic can wrap. A rejected encoding returns
PSA_ERROR_INVALID_SIGNATURE and psa_verify_hash() is not called.

Deriving both element offsets from the start of the buffer also removes the
odd-length fixup the old code needed to keep its running pointer in step. That
fixup only landed on the correct s length byte when r_len was
num_of_curve_bytes or one more — the two encodings a conforming signer
produces. For any other r_len the old code read the s length from the wrong
offset.

Only the PSA backend is affected. The TinyCrypt and mbedTLS ECDSA paths use
bootutil_decode_sig(), which is already bounded by an end pointer, and are
untouched.

Testing

Coverage of the changed function was confirmed by deliberately introducing a
syntax error in it: both PSA configurations fail to build, and sig-ecdsa (the
TinyCrypt path) builds fine as a negative control.

• cargo test --features sig-ecdsa-psa — 25/25
• cargo test --features "mbedtls-v4 sig-p384" — 25/25
• cargo test --features sig-ecdsa — 25/25

Note that the simulator only emits well-formed maximum-length DER, so the accept
path is exercised right at the new bounds, but the rejection branches are not
reached at runtime.

[Copilot]

Pull request overview

This PR hardens the PSA Crypto ECDSA signature parsing path by passing the caller-provided signature buffer length into the DER walker and validating tag/length fields before using them to compute offsets and memcpy ranges, preventing out-of-bounds reads on malformed inputs.

Changes:

• Change parse_signature_from_rfc5480_encoding() to take sig_len, return a PSA status code, and validate SEQUENCE/INTEGER tags and bounds before parsing.
• Update bootutil_ecdsa_verify() (PSA backend) to pass slen through and fail fast with PSA_ERROR_INVALID_SIGNATURE before calling psa_verify_hash() when parsing rejects the encoding.

Suppressed comments (2)

boot/bootutil/include/bootutil/crypto/ecdsa.h:391

• DER INTEGER lengths for r/s must be short-form and at least 1 byte. As written, r_len = sig[3] allows r_len == 0 (malformed INTEGER) and also does not explicitly reject long-form length bytes (high bit set), despite the function comment stating only short-form is handled.

    r_len = sig[3];
    if (r_len > seq_end - 6) {
        return (int)PSA_ERROR_INVALID_SIGNATURE;
    }

boot/bootutil/include/bootutil/crypto/ecdsa.h:401

• Similarly for s_len: a DER INTEGER must have a short-form length and at least 1 content byte. The current code accepts s_len == 0 and does not reject long-form length bytes (high bit set), so malformed encodings can slip through to psa_verify_hash().

    s_len = sig[4 + r_len + 1];
    s_off = 4 + r_len + 2;
    if (s_len > seq_end - s_off) {
        return (int)PSA_ERROR_INVALID_SIGNATURE;
    }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[de-nordic]

Addition of 2 or why length stored at sig[1] has to be <= 253 is not explained either. The entire comment is describing nothing that happens below it.

[de-nordic]

OK, how is the sig[3] related to sig[4]. from the comment? sig[4] is not used till line 414, i do not count line 410, because there it is offset by r_len (sig[3] and - num_of_curve_bytes.
The comment describes nothing that happens below it, at least till line 398? But there is s_off which is s_off used for offsetting source, and the comment sound more like we are constructing some buffer and need to have some extra 2 bytes for something?

[de-nordic]

Here is the part where we start to play with s, and it is missing description what is happening here. Yeah, I have figured out, without a comment.
It is like we are missing a description of the input buffer and how certain fields relate to the stream sequence in it, but the comments, like in line 385 and 374, do not help; they look like randomly placed.