Skip to content

Commit b79da0c

Browse files
authored
Merge pull request #12146 from ristohuhtala/mbed-coap-parser-buffer-overflow
mbed-coap buffer overflow fix
2 parents b8045fb + 82caa90 commit b79da0c

File tree

1 file changed

+34
-10
lines changed

1 file changed

+34
-10
lines changed

features/frameworks/mbed-coap/source/sn_coap_parser.c

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -310,16 +310,30 @@ static int8_t sn_coap_parser_options_parse(struct coap_s *handle, uint8_t **pack
310310
option_number = *(*packet_data_pptr + 1) + 13;
311311
(*packet_data_pptr)++;
312312
} else if (option_number == 14) {
313-
option_number = *(*packet_data_pptr + 2);
314-
option_number += (*(*packet_data_pptr + 1) << 8) + 269;
315-
(*packet_data_pptr) += 2;
313+
if (message_left >= 2){
314+
option_number = *(*packet_data_pptr + 2);
315+
option_number += (*(*packet_data_pptr + 1) << 8) + 269;
316+
(*packet_data_pptr) += 2;
317+
} else {
318+
/* packet_data_pptr would overflow! */
319+
tr_error("sn_coap_parser_options_parse - **packet_data_pptr overflow !");
320+
return -1;
321+
}
316322
}
317323
/* Option number 15 reserved for payload marker. This is handled as a error! */
318324
else if (option_number == 15) {
319325
tr_error("sn_coap_parser_options_parse - invalid option number(15)!");
320326
return -1;
321327
}
322328

329+
message_left = packet_len - ((*packet_data_pptr) - packet_data_start_ptr);
330+
331+
if (message_left == 0){
332+
/* packet_data_pptr would overflow! */
333+
tr_error("sn_coap_parser_options_parse - **packet_data_pptr overflow !");
334+
return -1;
335+
}
336+
323337
/* Add previous option to option delta and get option number */
324338
option_number += previous_option_number;
325339

@@ -328,9 +342,15 @@ static int8_t sn_coap_parser_options_parse(struct coap_s *handle, uint8_t **pack
328342
option_len = *(*packet_data_pptr + 1) + 13;
329343
(*packet_data_pptr)++;
330344
} else if (option_len == 14) {
331-
option_len = *(*packet_data_pptr + 2);
332-
option_len += (*(*packet_data_pptr + 1) << 8) + 269;
333-
(*packet_data_pptr) += 2;
345+
if (message_left >= 2){
346+
option_len = *(*packet_data_pptr + 2);
347+
option_len += (*(*packet_data_pptr + 1) << 8) + 269;
348+
(*packet_data_pptr) += 2;
349+
} else {
350+
/* packet_data_pptr would overflow! */
351+
tr_error("sn_coap_parser_options_parse - **packet_data_pptr overflow while resolving option length!");
352+
return -1;
353+
}
334354
}
335355
/* Option number length 15 is reserved for the future use - ERROR */
336356
else if (option_len == 15) {
@@ -340,6 +360,8 @@ static int8_t sn_coap_parser_options_parse(struct coap_s *handle, uint8_t **pack
340360

341361
message_left = packet_len - (*packet_data_pptr - packet_data_start_ptr);
342362

363+
364+
343365
/* * * Parse option itself * * */
344366
/* Some options are handled independently in own functions */
345367
previous_option_number = option_number;
@@ -366,6 +388,12 @@ static int8_t sn_coap_parser_options_parse(struct coap_s *handle, uint8_t **pack
366388
break;
367389
}
368390

391+
if (message_left < option_len){
392+
/* packet_data_pptr would overflow! */
393+
tr_error("sn_coap_parser_options_parse - **packet_data_pptr would overflow when parsing options!");
394+
return -1;
395+
}
396+
369397
/* Parse option */
370398
switch (option_number) {
371399
case COAP_OPTION_CONTENT_FORMAT:
@@ -400,9 +428,7 @@ static int8_t sn_coap_parser_options_parse(struct coap_s *handle, uint8_t **pack
400428
tr_error("sn_coap_parser_options_parse - COAP_OPTION_PROXY_URI allocation failed!");
401429
return -1;
402430
}
403-
404431
(*packet_data_pptr) += option_len;
405-
406432
break;
407433

408434
case COAP_OPTION_ETAG:
@@ -581,11 +607,9 @@ static int8_t sn_coap_parser_options_parse(struct coap_s *handle, uint8_t **pack
581607
if ((*packet_data_pptr - packet_data_start_ptr) > packet_len) {
582608
return -1;
583609
}
584-
585610
message_left = packet_len - (*packet_data_pptr - packet_data_start_ptr);
586611

587612
}
588-
589613
return 0;
590614
}
591615

0 commit comments

Comments
 (0)