pespin submitted this change.
sccp2sua: sccp_(long)ptr_part_consistent() Fix read buffer overflow if ptr in tail
Since we are dereferencing the otctet ptr, ptr must be at least 1 octet
before the tail pointer, otherwise we are reading outside of msgb data
boundaries.
Take the chance to rewrite a bit the functions to use variables to store
intermediate results, which makes it easier to follow the logic behind
the calculations following the example diagram in
ITU-T Q.713 Figure 2/Q.713.
Related: OS#7075
Reported-By: Tristan Madani <tristan@talencesecurity.com>
Change-Id: Ibc5da4d03fff9bd83e85945c28dded3608611ab7
---
M src/sccp2sua.c
1 file changed, 19 insertions(+), 14 deletions(-)
diff --git a/src/sccp2sua.c b/src/sccp2sua.c
index b8030a6..5460732 100644
--- a/src/sccp2sua.c
+++ b/src/sccp2sua.c
@@ -475,21 +475,26 @@
static bool sccp_ptr_part_consistent(const struct msgb *msg, const uint8_t *ptr_addr)
{
const uint8_t *ptr;
+ uint8_t offs;
+ uint8_t len;
/* check the address of the relative pointer is within msg */
- if (ptr_addr < msg->data || ptr_addr > msg->tail) {
+ if (ptr_addr < msg->data || ptr_addr >= msg->tail) {
LOGP(DLSUA, LOGL_ERROR, "ptr_addr outside msg boundary\n");
return false;
}
- ptr = ptr_addr + *ptr_addr;
- if (ptr > msg->tail) {
+ offs = *ptr_addr;
+ ptr = ptr_addr + offs;
+ if (ptr >= msg->tail) {
LOGP(DLSUA, LOGL_ERROR, "ptr points outside msg boundary\n");
return false;
}
/* at destination of relative pointer is the length */
- if (ptr + 1 + *ptr > msg->tail) {
+ len = *ptr;
+ ptr++;
+ if (ptr + len > msg->tail) {
LOGP(DLSUA, LOGL_ERROR, "ptr + len points outside msg boundary\n");
return false;
}
@@ -504,31 +509,31 @@
static bool sccp_longptr_part_consistent(const struct msgb *msg, const uint8_t *ptr_addr, bool len_is_long)
{
const uint8_t *ptr;
- uint8_t offs;
+ uint16_t offs;
+ uint8_t len_size = len_is_long ? 2 : 1;
uint16_t len;
/* check the address of the relative pointer is within msg */
- if (ptr_addr < msg->data || ptr_addr > msg->tail) {
+ if (ptr_addr < msg->data || (ptr_addr + sizeof(uint16_t)) > msg->tail) {
LOGP(DLSUA, LOGL_ERROR, "ptr_addr outside msg boundary\n");
return false;
}
+ offs = osmo_load16le(ptr_addr);
/* +1: Distance from MSB of pointer */
- ptr = ptr_addr + 1 + osmo_load16le(ptr_addr);
- if (ptr > msg->tail) {
+ ptr = ptr_addr + 1 + offs;
+ if (ptr + len_size > msg->tail) {
LOGP(DLSUA, LOGL_ERROR, "ptr %p points outside msg boundary %p\n", ptr, msg->tail);
return false;
}
/* at destination of relative pointer is the length */
- if (len_is_long) {
- offs = 2;
+ if (len_is_long)
len = osmo_load16le(ptr);
- } else {
- offs = 1;
+ else
len = *ptr;
- }
- if (ptr + offs + len > msg->tail) {
+ ptr += len_size;
+ if (ptr + len > msg->tail) {
LOGP(DLSUA, LOGL_ERROR, "ptr + len points outside msg boundary\n");
return false;
}
To view, visit change 43415. To unsubscribe, or for help writing mail filters, visit settings.