From 9d74efa30f7bc418b84fe92e03cc3d14d285d286 Mon Sep 17 00:00:00 2001 From: Ben Ford Date: Wed, 30 Nov 2022 12:02:12 -0600 Subject: [PATCH] pjproject: 2.13 security fixes Backports two security fixes (c4d3498 and 450baca) from pjproject 2.13. The first one was modified due to merge conflicts specifically with certified. ASTERISK-30338 Change-Id: I86fdc003d5d22cb66e7cc6dc3313a8194f27eb69 --- ...verflow-in-pjlib-scanner-and-pjmedia.patch | 307 ++++++++++++++++++ ...hen-parsing-message-as-a-STUN-client.patch | 44 +++ 2 files changed, 351 insertions(+) create mode 100644 third-party/pjproject/patches/0200-cert-18.9-potential-buffer-overflow-in-pjlib-scanner-and-pjmedia.patch create mode 100644 third-party/pjproject/patches/0201-potential-stack-buffer-overflow-when-parsing-message-as-a-STUN-client.patch diff --git a/third-party/pjproject/patches/0200-cert-18.9-potential-buffer-overflow-in-pjlib-scanner-and-pjmedia.patch b/third-party/pjproject/patches/0200-cert-18.9-potential-buffer-overflow-in-pjlib-scanner-and-pjmedia.patch new file mode 100644 index 0000000000..fa18860392 --- /dev/null +++ b/third-party/pjproject/patches/0200-cert-18.9-potential-buffer-overflow-in-pjlib-scanner-and-pjmedia.patch @@ -0,0 +1,307 @@ +From aefc5f83f7de651e3a37e7e1781bfaef46dab9c4 Mon Sep 17 00:00:00 2001 +From: Ben Ford +Date: Wed, 30 Nov 2022 11:28:16 -0600 +Subject: [PATCH] Merge pull request from GHSA-fq45-m3f7-3mhj + +* Initial patch + +* Use 'pj_scan_is_eof(scanner)' + +Co-authored-by: Aaron Lichtman + +* Use 'pj_scan_is_eof(scanner)' + +Co-authored-by: Aaron Lichtman + +* Use 'pj_scan_is_eof(scanner)' + +Co-authored-by: Aaron Lichtman + +* Use `!pj_scan_is_eof` instead of manually checking `scanner->curptr < scanner->end` + +Co-authored-by: Maksim Mukosey + +* Update pjlib-util/src/pjlib-util/scanner.c + +Co-authored-by: Aaron Lichtman + +* Update pjlib-util/src/pjlib-util/scanner.c + +Co-authored-by: Aaron Lichtman + +* Update pjlib-util/src/pjlib-util/scanner.c + +Co-authored-by: Aaron Lichtman + +* Revert '>=' back to '>' in pj_scan_stricmp_alnum() + +* Fix error compiles. + +Co-authered-by: sauwming +Co-authored-by: Nanang Izzuddin +Co-authored-by: Aaron Lichtman +Co-authored-by: Maksim Mukosey +--- + pjlib-util/src/pjlib-util/scanner.c | 41 +++++++++++++++++++---------- + pjmedia/src/pjmedia/rtp.c | 11 +++++--- + pjmedia/src/pjmedia/sdp.c | 24 ++++++++++------- + 3 files changed, 48 insertions(+), 28 deletions(-) + +diff --git a/pjlib-util/src/pjlib-util/scanner.c b/pjlib-util/src/pjlib-util/scanner.c +index c18b74c55..ea27bbec9 100644 +--- a/pjlib-util/src/pjlib-util/scanner.c ++++ b/pjlib-util/src/pjlib-util/scanner.c +@@ -195,7 +195,13 @@ PJ_DEF(void) pj_scan_skip_whitespace( pj_scanner *scanner ) + + PJ_DEF(void) pj_scan_skip_line( pj_scanner *scanner ) + { +- char *s = pj_ansi_strchr(scanner->curptr, '\n'); ++ char *s; ++ ++ if (pj_scan_is_eof(scanner)) { ++ return; ++ } ++ ++ s = pj_memchr(scanner->curptr, '\n', scanner->end - scanner->curptr); + if (!s) { + scanner->curptr = scanner->end; + } else { +@@ -264,8 +270,7 @@ PJ_DEF(void) pj_scan_get( pj_scanner *scanner, + + pj_assert(pj_cis_match(spec,0)==0); + +- /* EOF is detected implicitly */ +- if (!pj_cis_match(spec, *s)) { ++ if (pj_scan_is_eof(scanner) || !pj_cis_match(spec, *s)) { + pj_scan_syntax_err(scanner); + return; + } +@@ -299,8 +304,7 @@ PJ_DEF(void) pj_scan_get_unescape( pj_scanner *scanner, + /* Must not match character '%' */ + pj_assert(pj_cis_match(spec,'%')==0); + +- /* EOF is detected implicitly */ +- if (!pj_cis_match(spec, *s) && *s != '%') { ++ if (pj_scan_is_eof(scanner) || !pj_cis_match(spec, *s) && *s != '%') { + pj_scan_syntax_err(scanner); + return; + } +@@ -436,7 +440,9 @@ PJ_DEF(void) pj_scan_get_n( pj_scanner *scanner, + + scanner->curptr += N; + +- if (PJ_SCAN_IS_PROBABLY_SPACE(*scanner->curptr) && scanner->skip_ws) { ++ if (!pj_scan_is_eof(scanner) && ++ PJ_SCAN_IS_PROBABLY_SPACE(*scanner->curptr) && scanner->skip_ws) ++ { + pj_scan_skip_whitespace(scanner); + } + } +@@ -462,15 +468,16 @@ PJ_DEF(int) pj_scan_get_char( pj_scanner *scanner ) + + PJ_DEF(void) pj_scan_get_newline( pj_scanner *scanner ) + { +- if (!PJ_SCAN_IS_NEWLINE(*scanner->curptr)) { ++ if (pj_scan_is_eof(scanner) || !PJ_SCAN_IS_NEWLINE(*scanner->curptr)) { + pj_scan_syntax_err(scanner); + return; + } + ++ /* We have checked scanner->curptr validity above */ + if (*scanner->curptr == '\r') { + ++scanner->curptr; + } +- if (*scanner->curptr == '\n') { ++ if (!pj_scan_is_eof(scanner) && *scanner->curptr == '\n') { + ++scanner->curptr; + } + +@@ -515,7 +522,9 @@ PJ_DEF(void) pj_scan_get_until( pj_scanner *scanner, + + scanner->curptr = s; + +- if (PJ_SCAN_IS_PROBABLY_SPACE(*s) && scanner->skip_ws) { ++ if (!pj_scan_is_eof(scanner) && PJ_SCAN_IS_PROBABLY_SPACE(*s) && ++ scanner->skip_ws) ++ { + pj_scan_skip_whitespace(scanner); + } + } +@@ -539,7 +548,9 @@ PJ_DEF(void) pj_scan_get_until_ch( pj_scanner *scanner, + + scanner->curptr = s; + +- if (PJ_SCAN_IS_PROBABLY_SPACE(*s) && scanner->skip_ws) { ++ if (!pj_scan_is_eof(scanner) && PJ_SCAN_IS_PROBABLY_SPACE(*s) && ++ scanner->skip_ws) ++ { + pj_scan_skip_whitespace(scanner); + } + } +@@ -565,7 +576,9 @@ PJ_DEF(void) pj_scan_get_until_chr( pj_scanner *scanner, + + scanner->curptr = s; + +- if (PJ_SCAN_IS_PROBABLY_SPACE(*s) && scanner->skip_ws) { ++ if (!pj_scan_is_eof(scanner) && PJ_SCAN_IS_PROBABLY_SPACE(*s) && ++ scanner->skip_ws) ++ { + pj_scan_skip_whitespace(scanner); + } + } +@@ -580,7 +593,9 @@ PJ_DEF(void) pj_scan_advance_n( pj_scanner *scanner, + + scanner->curptr += N; + +- if (PJ_SCAN_IS_PROBABLY_SPACE(*scanner->curptr) && skip_ws) { ++ if (!pj_scan_is_eof(scanner) && ++ PJ_SCAN_IS_PROBABLY_SPACE(*scanner->curptr) && skip_ws) ++ { + pj_scan_skip_whitespace(scanner); + } + } +@@ -631,5 +646,3 @@ PJ_DEF(void) pj_scan_restore_state( pj_scanner *scanner, + scanner->line = state->line; + scanner->start_line = state->start_line; + } +- +- +diff --git a/pjmedia/src/pjmedia/rtp.c b/pjmedia/src/pjmedia/rtp.c +index 6c571010c..c987cd0ad 100644 +--- a/pjmedia/src/pjmedia/rtp.c ++++ b/pjmedia/src/pjmedia/rtp.c +@@ -183,6 +183,11 @@ PJ_DEF(pj_status_t) pjmedia_rtp_decode_rtp2( + /* Payload is located right after header plus CSRC */ + offset = sizeof(pjmedia_rtp_hdr) + ((*hdr)->cc * sizeof(pj_uint32_t)); + ++ /* Check that offset is less than packet size */ ++ if (offset >= pkt_len) { ++ return PJMEDIA_RTP_EINLEN; ++ } ++ + /* Decode RTP extension. */ + if ((*hdr)->x) { + dec_hdr->ext_hdr = (pjmedia_rtp_ext_hdr*)(((pj_uint8_t*)pkt) + offset); +@@ -195,8 +200,8 @@ PJ_DEF(pj_status_t) pjmedia_rtp_decode_rtp2( + dec_hdr->ext_len = 0; + } + +- /* Check that offset is less than packet size */ +- if (offset > pkt_len) ++ /* Check again that offset is still less than packet size */ ++ if (offset >= pkt_len) + return PJMEDIA_RTP_EINLEN; + + /* Find and set payload. */ +@@ -386,5 +391,3 @@ void pjmedia_rtp_seq_update( pjmedia_rtp_seq_session *sess, + seq_status->status.value = st.status.value; + } + } +- +- +diff --git a/pjmedia/src/pjmedia/sdp.c b/pjmedia/src/pjmedia/sdp.c +index c443d863f..f27a1a84f 100644 +--- a/pjmedia/src/pjmedia/sdp.c ++++ b/pjmedia/src/pjmedia/sdp.c +@@ -967,13 +967,13 @@ static void parse_version(pj_scanner *scanner, parse_context *ctx) + ctx->last_error = PJMEDIA_SDP_EINVER; + + /* check equal sign */ +- if (*(scanner->curptr+1) != '=') { ++ if (scanner->curptr+1 >= scanner->end || *(scanner->curptr+1) != '=') { + on_scanner_error(scanner); + return; + } + + /* check version is 0 */ +- if (*(scanner->curptr+2) != '0') { ++ if (scanner->curptr+2 >= scanner->end || *(scanner->curptr+2) != '0') { + on_scanner_error(scanner); + return; + } +@@ -990,7 +990,7 @@ static void parse_origin(pj_scanner *scanner, pjmedia_sdp_session *ses, + ctx->last_error = PJMEDIA_SDP_EINORIGIN; + + /* check equal sign */ +- if (*(scanner->curptr+1) != '=') { ++ if (scanner->curptr+1 >= scanner->end || *(scanner->curptr+1) != '=') { + on_scanner_error(scanner); + return; + } +@@ -1036,7 +1036,7 @@ static void parse_time(pj_scanner *scanner, pjmedia_sdp_session *ses, + ctx->last_error = PJMEDIA_SDP_EINTIME; + + /* check equal sign */ +- if (*(scanner->curptr+1) != '=') { ++ if (scanner->curptr+1 >= scanner->end || *(scanner->curptr+1) != '=') { + on_scanner_error(scanner); + return; + } +@@ -1064,7 +1064,7 @@ static void parse_generic_line(pj_scanner *scanner, pj_str_t *str, + ctx->last_error = PJMEDIA_SDP_EINSDP; + + /* check equal sign */ +- if (*(scanner->curptr+1) != '=') { ++ if ((scanner->curptr+1 >= scanner->end) || *(scanner->curptr+1) != '=') { + on_scanner_error(scanner); + return; + } +@@ -1133,7 +1133,7 @@ static void parse_media(pj_scanner *scanner, pjmedia_sdp_media *med, + ctx->last_error = PJMEDIA_SDP_EINMEDIA; + + /* check the equal sign */ +- if (*(scanner->curptr+1) != '=') { ++ if (scanner->curptr+1 >= scanner->end || *(scanner->curptr+1) != '=') { + on_scanner_error(scanner); + return; + } +@@ -1148,6 +1148,10 @@ static void parse_media(pj_scanner *scanner, pjmedia_sdp_media *med, + /* port */ + pj_scan_get(scanner, &cs_token, &str); + med->desc.port = (unsigned short)pj_strtoul(&str); ++ if (pj_scan_is_eof(scanner)) { ++ on_scanner_error(scanner); ++ return; ++ } + if (*scanner->curptr == '/') { + /* port count */ + pj_scan_get_char(scanner); +@@ -1159,7 +1163,7 @@ static void parse_media(pj_scanner *scanner, pjmedia_sdp_media *med, + } + + if (pj_scan_get_char(scanner) != ' ') { +- PJ_THROW(SYNTAX_ERROR); ++ on_scanner_error(scanner); + } + + /* transport */ +@@ -1167,7 +1171,7 @@ static void parse_media(pj_scanner *scanner, pjmedia_sdp_media *med, + + /* format list */ + med->desc.fmt_count = 0; +- while (*scanner->curptr == ' ') { ++ while (scanner->curptr < scanner->end && *scanner->curptr == ' ') { + pj_str_t fmt; + + pj_scan_get_char(scanner); +@@ -1207,7 +1211,7 @@ static pjmedia_sdp_attr *parse_attr( pj_pool_t *pool, pj_scanner *scanner, + attr = PJ_POOL_ALLOC_T(pool, pjmedia_sdp_attr); + + /* check equal sign */ +- if (*(scanner->curptr+1) != '=') { ++ if (scanner->curptr+1 >= scanner->end || *(scanner->curptr+1) != '=') { + on_scanner_error(scanner); + return NULL; + } +@@ -1226,7 +1230,7 @@ static pjmedia_sdp_attr *parse_attr( pj_pool_t *pool, pj_scanner *scanner, + pj_scan_get_char(scanner); + + /* get value */ +- if (*scanner->curptr != '\r' && *scanner->curptr != '\n') { ++ if (!pj_scan_is_eof(scanner) && *scanner->curptr != '\r' && *scanner->curptr != '\n') { + pj_scan_get_until_chr(scanner, "\r\n", &attr->value); + } else { + attr->value.ptr = NULL; +-- +2.25.1 + diff --git a/third-party/pjproject/patches/0201-potential-stack-buffer-overflow-when-parsing-message-as-a-STUN-client.patch b/third-party/pjproject/patches/0201-potential-stack-buffer-overflow-when-parsing-message-as-a-STUN-client.patch new file mode 100644 index 0000000000..76f02fccf3 --- /dev/null +++ b/third-party/pjproject/patches/0201-potential-stack-buffer-overflow-when-parsing-message-as-a-STUN-client.patch @@ -0,0 +1,44 @@ +From 450baca94f475345542c6953832650c390889202 Mon Sep 17 00:00:00 2001 +From: sauwming +Date: Tue, 7 Jun 2022 12:00:13 +0800 +Subject: [PATCH] Merge pull request from GHSA-26j7-ww69-c4qj + +--- + pjlib-util/src/pjlib-util/stun_simple.c | 7 ++++++- + 1 file changed, 6 insertions(+), 1 deletion(-) + +diff --git a/pjlib-util/src/pjlib-util/stun_simple.c b/pjlib-util/src/pjlib-util/stun_simple.c +index 722519584..d0549176d 100644 +--- a/pjlib-util/src/pjlib-util/stun_simple.c ++++ b/pjlib-util/src/pjlib-util/stun_simple.c +@@ -54,6 +54,7 @@ PJ_DEF(pj_status_t) pjstun_parse_msg( void *buf, pj_size_t buf_len, + { + pj_uint16_t msg_type, msg_len; + char *p_attr; ++ int attr_max_cnt = PJ_ARRAY_SIZE(msg->attr); + + PJ_CHECK_STACK(); + +@@ -83,7 +84,7 @@ PJ_DEF(pj_status_t) pjstun_parse_msg( void *buf, pj_size_t buf_len, + msg->attr_count = 0; + p_attr = (char*)buf + sizeof(pjstun_msg_hdr); + +- while (msg_len > 0) { ++ while (msg_len > 0 && msg->attr_count < attr_max_cnt) { + pjstun_attr_hdr **attr = &msg->attr[msg->attr_count]; + pj_uint32_t len; + pj_uint16_t attr_type; +@@ -111,6 +112,10 @@ PJ_DEF(pj_status_t) pjstun_parse_msg( void *buf, pj_size_t buf_len, + p_attr += len; + ++msg->attr_count; + } ++ if (msg->attr_count == attr_max_cnt) { ++ PJ_LOG(4, (THIS_FILE, "Warning: max number attribute %d reached.", ++ attr_max_cnt)); ++ } + + return PJ_SUCCESS; + } +-- +2.25.1 +