From cbb7585e7daf361f1e2919f1080ac303a8cdf610 Mon Sep 17 00:00:00 2001 From: Ben Ford Date: Tue, 29 Nov 2022 14:02:26 -0600 Subject: [PATCH] pjproject: 2.13 security fixes Backports two security fixes (c4d3498 and 450baca) from pjproject 2.13. ASTERISK-30338 Change-Id: I86fdc003d5d22cb66e7cc6dc3313a8194f27eb69 --- ...verflow-in-pjlib-scanner-and-pjmedia.patch | 306 ++++++++++++++++++ ...hen-parsing-message-as-a-STUN-client.patch | 44 +++ 2 files changed, 350 insertions(+) create mode 100644 third-party/pjproject/patches/0200-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-potential-buffer-overflow-in-pjlib-scanner-and-pjmedia.patch b/third-party/pjproject/patches/0200-potential-buffer-overflow-in-pjlib-scanner-and-pjmedia.patch new file mode 100644 index 0000000000..cf4359a817 --- /dev/null +++ b/third-party/pjproject/patches/0200-potential-buffer-overflow-in-pjlib-scanner-and-pjmedia.patch @@ -0,0 +1,306 @@ +From c4d34984ec92b3d5252a7d5cddd85a1d3a8001ae Mon Sep 17 00:00:00 2001 +From: sauwming +Date: Mon, 3 Oct 2022 08:07:22 +0800 +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-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 a54edf2d8..6541bbae3 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_memchr(scanner->curptr, '\n', scanner->end - scanner->curptr); ++ 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); + } + } +@@ -467,15 +473,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; + } + +@@ -520,7 +527,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); + } + } +@@ -544,7 +553,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); + } + } +@@ -570,7 +581,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); + } + } +@@ -585,7 +598,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); + } + } +@@ -636,5 +651,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 18917f18b..d29348cc5 100644 +--- a/pjmedia/src/pjmedia/rtp.c ++++ b/pjmedia/src/pjmedia/rtp.c +@@ -188,6 +188,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) { + if (offset + sizeof (pjmedia_rtp_ext_hdr) > (unsigned)pkt_len) +@@ -202,8 +207,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. */ +@@ -393,5 +398,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 3905c2f52..647f49e13 100644 +--- a/pjmedia/src/pjmedia/sdp.c ++++ b/pjmedia/src/pjmedia/sdp.c +@@ -983,13 +983,13 @@ static void parse_version(pj_scanner *scanner, + 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; + } +@@ -1006,7 +1006,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; + } +@@ -1052,7 +1052,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; + } +@@ -1080,7 +1080,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; + } +@@ -1149,7 +1149,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; + } +@@ -1164,6 +1164,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); +@@ -1175,7 +1179,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 */ +@@ -1183,7 +1187,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); +@@ -1223,7 +1227,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; + } +@@ -1242,7 +1246,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 +