pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/43415?usp=email )
Change subject: sccp2sua: sccp_(long)ptr_part_consistent() Fix read buffer overflow if ptr in tail ...................................................................... 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 <[email protected]> Change-Id: Ibc5da4d03fff9bd83e85945c28dded3608611ab7 --- M src/sccp2sua.c 1 file changed, 19 insertions(+), 14 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-sigtran refs/changes/15/43415/1 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 https://gerrit.osmocom.org/c/libosmo-sigtran/+/43415?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: libosmo-sigtran Gerrit-Branch: master Gerrit-Change-Id: Ibc5da4d03fff9bd83e85945c28dded3608611ab7 Gerrit-Change-Number: 43415 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <[email protected]>
