From 01cd003b46fa2ebea4d9be5475b11217eb4c11be Mon Sep 17 00:00:00 2001 From: James Bonfield Date: Mon, 12 Jan 2026 10:16:29 +0000 Subject: [PATCH] Added extra overrun detection in BYTE_ARRAY_LEN decoder. Although the input specifies the maximum size of 'out' in *out_side (which is subsequently replaced by the actual size), this wasn't checked for when calling byte_array_len.len_codec->decode to fetch the length field. The obvious fix did trigger other problems however. We've been routinely calling this with incorrect parameters when decoding aux tags. Eg XA:i encoded as XAs (short) would specify out_size of 1, as we're attempting to decode a single item. This is the standard way all of our decoders work - indicating the number of things to decode rather than their byte lengths. However by definition this is a "byte array" so we need to compensate for that before calling the decode function. It wasn't required before (as we didn't check). --- cram/cram_codecs.c | 21 +++++++++++++-------- cram/cram_decode.c | 24 ++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/cram/cram_codecs.c b/cram/cram_codecs.c index 5560192ed..6364fae52 100644 --- a/cram/cram_codecs.c +++ b/cram/cram_codecs.c @@ -3357,14 +3357,19 @@ int cram_byte_array_len_decode(cram_slice *slice, cram_codec *c, int32_t len = 0, one = 1; int r; - r = c->u.byte_array_len.len_codec->decode(slice, c->u.byte_array_len.len_codec, - in, (char *)&len, &one); - //printf("ByteArray Len=%d\n", len); - - if (!r && c->u.byte_array_len.val_codec && len >= 0) { - r = c->u.byte_array_len.val_codec->decode(slice, - c->u.byte_array_len.val_codec, - in, out, &len); + cram_codec *len_codec = c->u.byte_array_len.len_codec; + cram_codec *val_codec = c->u.byte_array_len.val_codec; + + r = len_codec->decode(slice, len_codec, in, (char *)&len, &one); + if (len < 0 || (len > *out_size && + !(val_codec->codec == E_EXTERNAL && + val_codec->u.external.type == E_BYTE_ARRAY_BLOCK))) { + fprintf(stderr, "Attempted overrun detected in %s\n", __FUNCTION__); + return -1; + } + + if (!r && val_codec) { + r = val_codec->decode(slice, val_codec, in, out, &len); } else { return -1; } diff --git a/cram/cram_decode.c b/cram/cram_decode.c index 38774414f..d91b9bc53 100644 --- a/cram/cram_decode.c +++ b/cram/cram_decode.c @@ -1948,6 +1948,23 @@ static int cram_decode_aux_1_0(cram_container *c, cram_slice *s, return -1; } +// Derived from sam.c aux_type2size +static inline int aux_ele_size(uint8_t type) +{ + switch (type) { + case 'A': case 'c': case 'C': + return 1; + case 's': case 'S': + return 2; + case 'i': case 'I': case 'f': + return 4; + case 'd': + return 8; + default: + return 1; + } +} + // has_MD and has_NM are filled out with 0 for none present, // 1 for present and verbatim, and -pos for present as placeholder // (MD*, NM*) to be generated and filled out at offset +pos. @@ -2039,6 +2056,13 @@ static int cram_decode_aux(cram_fd *fd, BLOCK_APPEND(s->aux_blk, (char *)tag_data, 3); if (!m->codec) return -1; + if (m->codec->codec == E_BYTE_ARRAY_LEN || + m->codec->codec == E_BYTE_ARRAY_STOP) + // NB we don't know the maximum length for B arrays yet, + // but we're using BYTE_ARRAY_BLOCK encodings so they're auto- + // resizing arrays that cannot overflow. The codec handles this + // check for us. + out_sz *= aux_ele_size(TN[-1]); r |= m->codec->decode(s, m->codec, blk, (char *)s->aux_blk, &out_sz); if (r) break; cr->aux_size += out_sz + 3;