fixeria has uploaded this change for review. (
https://gerrit.osmocom.org/c/libosmocore/+/43235?usp=email )
Change subject: gsm0480: fix out-of-bounds reads in
parse_ss_{invoke,return_result}()
......................................................................
gsm0480: fix out-of-bounds reads in parse_ss_{invoke,return_result}()
Both Facility component parsers checked 'offset' against 'length' one
(or two) bytes short of the index they went on to dereference, and
parse_ss_invoke() never re-validated 'offset' after skipping the
optional Linked ID, whose skip length is attacker-controlled. The same
off-by-one on the operation-code path also let "length - offset - 3"
underflow a uint16_t, handing parse_process_uss_req() a bogus,
oversized length that bypassed its own bounds check and grew the
over-read into a memcpy() into req->ussd_data/req->ussd_text.
Tighten each guard to cover the index actually dereferenced, and
re-check 'offset' against 'length' after the Linked ID skip and after
the post-SEQUENCE-tag increment in parse_ss_return_result().
Change-Id: I59fe4df8045dbe1e2b9509330527408b84abf2e4
Reported-By: Adam Bedard <[email protected]>
Fixes: OS#7051
---
M src/gsm/gsm0480.c
1 file changed, 15 insertions(+), 6 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/35/43235/1
diff --git a/src/gsm/gsm0480.c b/src/gsm/gsm0480.c
index a8eac6b..5db5bed 100644
--- a/src/gsm/gsm0480.c
+++ b/src/gsm/gsm0480.c
@@ -548,17 +548,24 @@
offset = invoke_data[1] + 2;
req->invoke_id = invoke_data[2];
- /* look ahead once */
- if (offset + 1 > length)
+ /* look ahead once: need invoke_data[offset] and, if it turns out to be
+ * the optional Linked ID tag, invoke_data[offset+1] as well */
+ if (offset + 2 > length)
return 0;
/* optional part */
- if (invoke_data[offset] == GSM0480_COMPIDTAG_LINKED_ID)
+ if (invoke_data[offset] == GSM0480_COMPIDTAG_LINKED_ID) {
offset += invoke_data[offset+1] + 2; /* skip over it */
+ /* offset moved by an attacker-controlled amount: re-validate */
+ if (offset >= length)
+ return 0;
+ }
+
/* mandatory part */
if (invoke_data[offset] == GSM0480_OPERATION_CODE) {
- if (offset + 2 > length)
+ /* need invoke_data[offset+2] below, and length - offset - 3
must not underflow */
+ if (offset + 3 > length)
return 0;
uint8_t operation_code = invoke_data[offset+2];
req->opcode = operation_code;
@@ -624,10 +631,12 @@
if (rr_data[offset] != GSM_0480_SEQUENCE_TAG)
return 0;
- if (offset + 2 > length)
+ offset += 2;
+
+ /* need rr_data[offset+2] below, and length - offset - 3 must not
underflow */
+ if (offset + 3 > length)
return 0;
- offset += 2;
operation_code = rr_data[offset + 2];
req->opcode = operation_code;
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/43235?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I59fe4df8045dbe1e2b9509330527408b84abf2e4
Gerrit-Change-Number: 43235
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <[email protected]>
