neels has uploaded this change for review.

View Change

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;

To view, visit change 37984. To unsubscribe, or for help writing mail filters, visit settings.