neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/libasn1c/+/37984?usp=email )
Change subject: coverity CID#57681 ......................................................................
coverity CID#57681
Make sure that bits_unused cannot subtract more bits than present in st->size.
Especially when st->size == 0, this ensures that sizeinunits is also 0, and that a st->size == 0 hence never enters the while (sizeinunits) loop.
Change-Id: Ic8e3f5e24541989b79826de2942e5e6560079028 --- M src/OCTET_STRING.c 1 file changed, 9 insertions(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/libasn1c refs/changes/84/37984/1
diff --git a/src/OCTET_STRING.c b/src/OCTET_STRING.c index 3e424e7..9d2e817 100644 --- a/src/OCTET_STRING.c +++ b/src/OCTET_STRING.c @@ -1859,6 +1859,7 @@ unsigned int unit_bits; unsigned int canonical_unit_bits; unsigned int sizeinunits; + unsigned int unused; const uint8_t *buf; int ret; enum { @@ -1888,7 +1889,11 @@ case ASN_OSUBV_BIT: canonical_unit_bits = unit_bits = 1; bpc = OS__BPC_BIT; - sizeinunits = st->size * 8 - (st->bits_unused & 0x07); + sizeinunits = st->size * 8; + /* make sure sizeinunits cannot wrap past zero (especially when st->size == 0). */ + unused = st->bits_unused & 0x07; + if (unused <= sizeinunits) + sizeinunits -= unused; ASN_DEBUG("BIT STRING of %d bytes", sizeinunits); break; @@ -1994,6 +1999,9 @@ maySave, bpc, unit_bits, cval->lower_bound, cval->upper_bound, pc); } else { + /* coverity CID#57681: st->buf can be NULL, but above, we've verified that when st->buf == NULL, + * then also st->size == 0, and hence sizeinunits == 0, hence this 'while' will skip entirely, + * and hence it is safe to dereference buf here. */ ret = per_put_many_bits(po, buf, maySave * unit_bits); } if(ret) _ASN_ENCODE_FAILED;