fixeria has uploaded this change for review.

View Change

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';


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

Gerrit-MessageType: newchange
Gerrit-Project: libosmo-sigtran
Gerrit-Branch: master
Gerrit-Change-Id: I39c1e8acbec371a27c7871799619695d77b4d61e
Gerrit-Change-Number: 43348
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanitskiy@sysmocom.de>