| Message ID | 1461328875-8253-1-git-send-email-sergey.kostanbaev@gmail.com |
|---|---|
| State | Not Applicable |
| Headers | show |
> On 22 Apr 2016, at 14:41, Sergey Kostanbaev <sergey.kostanbaev@gmail.com> wrote: > > From: Sergey Kostanbaev <Sergey.Kostanbaev@gmail.com> > > --- > src/gsm/gsm0480.c | 6 ------ > 1 file changed, 6 deletions(-) > > diff --git a/src/gsm/gsm0480.c b/src/gsm/gsm0480.c > index 8963b78..55bddd5 100644 > --- a/src/gsm/gsm0480.c > +++ b/src/gsm/gsm0480.c > @@ -248,12 +248,6 @@ int gsm0480_decode_ss_request(const struct gsm48_hdr *hdr, uint16_t len, > struct ss_request *req) > { > int rc = 0; > - > - if (len < sizeof(*hdr) + 2) { > - LOGP(0, LOGL_DEBUG, "SS Request is too short.\n"); > - return 0; > - } > - > if (gsm48_hdr_pdisc(hdr) == GSM48_PDISC_NC_SS) { > req->transaction_id = hdr->proto_discr & 0x70; > rc = parse_ss(hdr, len, req); static int parse_ss_info_elements(const uint8_t *ss_ie, uint16_t len, struct ss_request *req) { int rc = -1; /* Information Element Identifier - table 3.2 & GSM 04.08 section 10.5 */ uint8_t iei; uint8_t iei_length; iei = ss_ie[0]; iei_length = ss_ie[1]; /* If the data does not fit, report an error */ if (len - 2 < iei_length) return 0; so this code works with uint16_t and assumes there is at least two bytes after the header. I don't know best practices for integer underflow/overflow (shall we use ssize_t)? First we need a testcase Second we need to remove the assumptions about at least two bytes in payload. holger
On Fri, Apr 22, 2016 at 02:41:12PM +0200, Sergey Kostanbaev wrote: > From: Sergey Kostanbaev <Sergey.Kostanbaev@gmail.com> > > --- > src/gsm/gsm0480.c | 6 ------ > 1 file changed, 6 deletions(-) > > diff --git a/src/gsm/gsm0480.c b/src/gsm/gsm0480.c > index 8963b78..55bddd5 100644 > --- a/src/gsm/gsm0480.c > +++ b/src/gsm/gsm0480.c > @@ -248,12 +248,6 @@ int gsm0480_decode_ss_request(const struct gsm48_hdr *hdr, uint16_t len, > struct ss_request *req) > { > int rc = 0; > - > - if (len < sizeof(*hdr) + 2) { > - LOGP(0, LOGL_DEBUG, "SS Request is too short.\n"); > - return 0; > - } > - > if (gsm48_hdr_pdisc(hdr) == GSM48_PDISC_NC_SS) { I believe this should only remove the "+ 2" and still check for if (len < sizeof(*hdr)) { return 0; } since the code continues to dereference hdr, or is this the explicit responsibility of the caller? ~Neels
diff --git a/src/gsm/gsm0480.c b/src/gsm/gsm0480.c index 8963b78..55bddd5 100644 --- a/src/gsm/gsm0480.c +++ b/src/gsm/gsm0480.c @@ -248,12 +248,6 @@ int gsm0480_decode_ss_request(const struct gsm48_hdr *hdr, uint16_t len, struct ss_request *req) { int rc = 0; - - if (len < sizeof(*hdr) + 2) { - LOGP(0, LOGL_DEBUG, "SS Request is too short.\n"); - return 0; - } - if (gsm48_hdr_pdisc(hdr) == GSM48_PDISC_NC_SS) { req->transaction_id = hdr->proto_discr & 0x70; rc = parse_ss(hdr, len, req);
From: Sergey Kostanbaev <Sergey.Kostanbaev@gmail.com> --- src/gsm/gsm0480.c | 6 ------ 1 file changed, 6 deletions(-)