fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/43348?usp=email )
Change subject: sua: refactor sua_parse_gt() digit loop ......................................................................
sua: refactor sua_parse_gt() digit loop
The digit-parsing loop in sua_parse_gt() bounded output writes to gt->digits[] via mid-loop break checks against the clamped num_digits, while the loop itself iterated over datalen-8. Although provably safe, Coverity cannot correlate the break conditions with the loop bound and flags a false-positive overrun. Rewrite the loop to iterate directly over num_digits, so the bound on writes into gt->digits[] is syntactically obvious. No functional change.
Change-Id: I39c1e8acbec371a27c7871799619695d77b4d61e Related: OS#7046, CID#562801 --- M src/sua.c 1 file changed, 12 insertions(+), 8 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-sigtran refs/changes/48/43348/1
diff --git a/src/sua.c b/src/sua.c index 35c4a8e..7c6d57f 100644 --- a/src/sua.c +++ b/src/sua.c @@ -412,16 +412,20 @@ rc = -ENOSPC; }
- /* parse digits */ + /* parse digits: bounded by num_digits (already clamped above), so this + * can never write more than sizeof(gt->digits)-1 nibbles into gt->digits[] */ out_digits = gt->digits; - for (i = 0; i < datalen-8; i++) { - uint8_t byte = data[8+i]; - *out_digits++ = osmo_bcd2char(byte & 0x0F); - if (out_digits - gt->digits >= num_digits) + for (i = 0; i < num_digits; i++) { + unsigned int byte_off = 8 + i / 2; + uint8_t byte; + + if (byte_off >= datalen) break; - *out_digits++ = osmo_bcd2char(byte >> 4); - if (out_digits - gt->digits >= num_digits) - break; + byte = data[byte_off]; + if (i & 1) + *out_digits++ = osmo_bcd2char(byte >> 4); + else + *out_digits++ = osmo_bcd2char(byte & 0x0F); } *out_digits++ = '\0';