Skip to content

Commit 11559e4

Browse files
authored
Merge pull request from GHSA-vhxv-phmx-g52q
* Prevent OOB read/write when parsing RTCP FB RPSI * Add log information * Modification based on comments.
1 parent 856f87c commit 11559e4

File tree

3 files changed

+73
-38
lines changed

3 files changed

+73
-38
lines changed

Diff for: pjmedia/include/pjmedia/rtcp.h

+11
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,15 @@ typedef struct pjmedia_rtcp_common
115115
} pjmedia_rtcp_common;
116116

117117

118+
/**
119+
* RTCP feedback common header.
120+
*/
121+
typedef struct pjmedia_rtcp_fb_common
122+
{
123+
pjmedia_rtcp_common rtcp_common;
124+
pj_uint32_t ssrc_src; /**< SSRC media source */
125+
} pjmedia_rtcp_fb_common;
126+
118127
/**
119128
* This structure declares default RTCP packet (SR) that is sent by pjmedia.
120129
* Incoming RTCP packet may have different format, and must be parsed
@@ -234,6 +243,8 @@ typedef struct pjmedia_rtcp_session
234243
char *name; /**< Name identification. */
235244
pjmedia_rtcp_sr_pkt rtcp_sr_pkt;/**< Cached RTCP SR packet. */
236245
pjmedia_rtcp_rr_pkt rtcp_rr_pkt;/**< Cached RTCP RR packet. */
246+
pjmedia_rtcp_fb_common rtcp_fb_com;/**< Cached RTCP feedback common
247+
header packet. */
237248

238249
pjmedia_rtp_seq_session seq_ctrl; /**< RTCP sequence number control. */
239250
unsigned rtp_last_ts;/**< Last timestamp in RX RTP pkt. */

Diff for: pjmedia/src/pjmedia/rtcp.c

+5
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,11 @@ PJ_DEF(void) pjmedia_rtcp_init2( pjmedia_rtcp_session *sess,
242242
sess->rtcp_rr_pkt.common.pt = RTCP_RR;
243243
sess->rtcp_rr_pkt.common.length = pj_htons(7);
244244

245+
/* Copy to RTCP FB common header */
246+
pj_memcpy(&sess->rtcp_fb_com, &sr_pkt->common,
247+
sizeof(pjmedia_rtcp_common));
248+
sess->rtcp_fb_com.ssrc_src = 0;
249+
245250
/* Get time and timestamp base and frequency */
246251
pj_gettimeofday(&now);
247252
sess->tv_base = now;

Diff for: pjmedia/src/pjmedia/rtcp_fb.c

+57-38
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ PJ_DEF(pj_status_t) pjmedia_rtcp_fb_build_nack(
4343
unsigned nack_cnt,
4444
const pjmedia_rtcp_fb_nack nack[])
4545
{
46-
pjmedia_rtcp_common *hdr;
46+
pjmedia_rtcp_fb_common *hdr;
4747
pj_uint8_t *p;
4848
unsigned len, i;
4949

@@ -54,11 +54,11 @@ PJ_DEF(pj_status_t) pjmedia_rtcp_fb_build_nack(
5454
return PJ_ETOOSMALL;
5555

5656
/* Build RTCP-FB NACK header */
57-
hdr = (pjmedia_rtcp_common*)buf;
58-
pj_memcpy(hdr, &session->rtcp_rr_pkt.common, sizeof(*hdr));
59-
hdr->pt = RTCP_RTPFB;
60-
hdr->count = 1; /* FMT = 1 */
61-
hdr->length = pj_htons((pj_uint16_t)(len/4 - 1));
57+
hdr = (pjmedia_rtcp_fb_common*)buf;
58+
pj_memcpy(hdr, &session->rtcp_fb_com, sizeof(*hdr));
59+
hdr->rtcp_common.pt = RTCP_RTPFB;
60+
hdr->rtcp_common.count = 1; /* FMT = 1 */
61+
hdr->rtcp_common.length = pj_htons((pj_uint16_t)(len/4 - 1));
6262

6363
/* Build RTCP-FB NACK FCI */
6464
p = (pj_uint8_t*)hdr + sizeof(*hdr);
@@ -86,7 +86,7 @@ PJ_DEF(pj_status_t) pjmedia_rtcp_fb_build_pli(
8686
void *buf,
8787
pj_size_t *length)
8888
{
89-
pjmedia_rtcp_common *hdr;
89+
pjmedia_rtcp_fb_common *hdr;
9090
unsigned len;
9191

9292
PJ_ASSERT_RETURN(session && buf && length, PJ_EINVAL);
@@ -96,11 +96,11 @@ PJ_DEF(pj_status_t) pjmedia_rtcp_fb_build_pli(
9696
return PJ_ETOOSMALL;
9797

9898
/* Build RTCP-FB PLI header */
99-
hdr = (pjmedia_rtcp_common*)buf;
100-
pj_memcpy(hdr, &session->rtcp_rr_pkt.common, sizeof(*hdr));
101-
hdr->pt = RTCP_PSFB;
102-
hdr->count = 1; /* FMT = 1 */
103-
hdr->length = pj_htons((pj_uint16_t)(len/4 - 1));
99+
hdr = (pjmedia_rtcp_fb_common*)buf;
100+
pj_memcpy(hdr, &session->rtcp_fb_com, sizeof(*hdr));
101+
hdr->rtcp_common.pt = RTCP_PSFB;
102+
hdr->rtcp_common.count = 1; /* FMT = 1 */
103+
hdr->rtcp_common.length = pj_htons((pj_uint16_t)(len/4 - 1));
104104

105105
/* Finally */
106106
*length = len;
@@ -119,7 +119,7 @@ PJ_DEF(pj_status_t) pjmedia_rtcp_fb_build_sli(
119119
unsigned sli_cnt,
120120
const pjmedia_rtcp_fb_sli sli[])
121121
{
122-
pjmedia_rtcp_common *hdr;
122+
pjmedia_rtcp_fb_common *hdr;
123123
pj_uint8_t *p;
124124
unsigned len, i;
125125

@@ -130,11 +130,11 @@ PJ_DEF(pj_status_t) pjmedia_rtcp_fb_build_sli(
130130
return PJ_ETOOSMALL;
131131

132132
/* Build RTCP-FB SLI header */
133-
hdr = (pjmedia_rtcp_common*)buf;
134-
pj_memcpy(hdr, &session->rtcp_rr_pkt.common, sizeof(*hdr));
135-
hdr->pt = RTCP_PSFB;
136-
hdr->count = 2; /* FMT = 2 */
137-
hdr->length = pj_htons((pj_uint16_t)(len/4 - 1));
133+
hdr = (pjmedia_rtcp_fb_common*)buf;
134+
pj_memcpy(hdr, &session->rtcp_fb_com, sizeof(*hdr));
135+
hdr->rtcp_common.pt = RTCP_PSFB;
136+
hdr->rtcp_common.count = 2; /* FMT = 2 */
137+
hdr->rtcp_common.length = pj_htons((pj_uint16_t)(len/4 - 1));
138138

139139
/* Build RTCP-FB SLI FCI */
140140
p = (pj_uint8_t*)hdr + sizeof(*hdr);
@@ -166,7 +166,7 @@ PJ_DEF(pj_status_t) pjmedia_rtcp_fb_build_rpsi(
166166
pj_size_t *length,
167167
const pjmedia_rtcp_fb_rpsi *rpsi)
168168
{
169-
pjmedia_rtcp_common *hdr;
169+
pjmedia_rtcp_fb_common *hdr;
170170
pj_uint8_t *p;
171171
unsigned bitlen, padlen, len;
172172

@@ -179,11 +179,11 @@ PJ_DEF(pj_status_t) pjmedia_rtcp_fb_build_rpsi(
179179
return PJ_ETOOSMALL;
180180

181181
/* Build RTCP-FB RPSI header */
182-
hdr = (pjmedia_rtcp_common*)buf;
183-
pj_memcpy(hdr, &session->rtcp_rr_pkt.common, sizeof(*hdr));
184-
hdr->pt = RTCP_PSFB;
185-
hdr->count = 3; /* FMT = 3 */
186-
hdr->length = pj_htons((pj_uint16_t)(len/4 - 1));
182+
hdr = (pjmedia_rtcp_fb_common*)buf;
183+
pj_memcpy(hdr, &session->rtcp_fb_com, sizeof(*hdr));
184+
hdr->rtcp_common.pt = RTCP_PSFB;
185+
hdr->rtcp_common.count = 3; /* FMT = 3 */
186+
hdr->rtcp_common.length = pj_htons((pj_uint16_t)(len/4 - 1));
187187

188188
/* Build RTCP-FB RPSI FCI */
189189
p = (pj_uint8_t*)hdr + sizeof(*hdr);
@@ -620,18 +620,18 @@ PJ_DEF(pj_status_t) pjmedia_rtcp_fb_parse_nack(
620620
unsigned *nack_cnt,
621621
pjmedia_rtcp_fb_nack nack[])
622622
{
623-
pjmedia_rtcp_common *hdr = (pjmedia_rtcp_common*) buf;
623+
pjmedia_rtcp_fb_common *hdr = (pjmedia_rtcp_fb_common*) buf;
624624
pj_uint8_t *p;
625625
unsigned cnt, i;
626626

627627
PJ_ASSERT_RETURN(buf && nack_cnt && nack, PJ_EINVAL);
628-
PJ_ASSERT_RETURN(length >= sizeof(pjmedia_rtcp_common), PJ_ETOOSMALL);
628+
PJ_ASSERT_RETURN(length >= sizeof(pjmedia_rtcp_fb_common), PJ_ETOOSMALL);
629629

630630
/* Generic NACK uses pt==RTCP_RTPFB and FMT==1 */
631-
if (hdr->pt != RTCP_RTPFB || hdr->count != 1)
631+
if (hdr->rtcp_common.pt != RTCP_RTPFB || hdr->rtcp_common.count != 1)
632632
return PJ_ENOTFOUND;
633633

634-
cnt = pj_ntohs((pj_uint16_t)hdr->length);
634+
cnt = pj_ntohs((pj_uint16_t)hdr->rtcp_common.length);
635635
if (cnt > 2) cnt -= 2; else cnt = 0;
636636
if (length < (cnt+3)*4)
637637
return PJ_ETOOSMALL;
@@ -661,15 +661,15 @@ PJ_DEF(pj_status_t) pjmedia_rtcp_fb_parse_pli(
661661
const void *buf,
662662
pj_size_t length)
663663
{
664-
pjmedia_rtcp_common *hdr = (pjmedia_rtcp_common*) buf;
664+
pjmedia_rtcp_fb_common *hdr = (pjmedia_rtcp_fb_common*) buf;
665665

666666
PJ_ASSERT_RETURN(buf, PJ_EINVAL);
667667

668668
if (length < 12)
669669
return PJ_ETOOSMALL;
670670

671671
/* PLI uses pt==RTCP_PSFB and FMT==1 */
672-
if (hdr->pt != RTCP_PSFB || hdr->count != 1)
672+
if (hdr->rtcp_common.pt != RTCP_PSFB || hdr->rtcp_common.count != 1)
673673
return PJ_ENOTFOUND;
674674

675675
return PJ_SUCCESS;
@@ -686,18 +686,18 @@ PJ_DEF(pj_status_t) pjmedia_rtcp_fb_parse_sli(
686686
unsigned *sli_cnt,
687687
pjmedia_rtcp_fb_sli sli[])
688688
{
689-
pjmedia_rtcp_common *hdr = (pjmedia_rtcp_common*) buf;
689+
pjmedia_rtcp_fb_common *hdr = (pjmedia_rtcp_fb_common*) buf;
690690
pj_uint8_t *p;
691691
unsigned cnt, i;
692692

693693
PJ_ASSERT_RETURN(buf && sli_cnt && sli, PJ_EINVAL);
694-
PJ_ASSERT_RETURN(length >= sizeof(pjmedia_rtcp_common), PJ_ETOOSMALL);
694+
PJ_ASSERT_RETURN(length >= sizeof(pjmedia_rtcp_fb_common), PJ_ETOOSMALL);
695695

696696
/* PLI uses pt==RTCP_PSFB and FMT==2 */
697-
if (hdr->pt != RTCP_PSFB || hdr->count != 2)
697+
if (hdr->rtcp_common.pt != RTCP_PSFB || hdr->rtcp_common.count != 2)
698698
return PJ_ENOTFOUND;
699699

700-
cnt = pj_ntohs((pj_uint16_t)hdr->length) - 2;
700+
cnt = pj_ntohs((pj_uint16_t)hdr->rtcp_common.length) - 2;
701701
if (length < (cnt+3)*4)
702702
return PJ_ETOOSMALL;
703703

@@ -730,24 +730,43 @@ PJ_DEF(pj_status_t) pjmedia_rtcp_fb_parse_rpsi(
730730
pj_size_t length,
731731
pjmedia_rtcp_fb_rpsi *rpsi)
732732
{
733-
pjmedia_rtcp_common *hdr = (pjmedia_rtcp_common*) buf;
733+
pjmedia_rtcp_fb_common *hdr = (pjmedia_rtcp_fb_common*) buf;
734734
pj_uint8_t *p;
735735
pj_uint8_t padlen;
736736
pj_size_t rpsi_len;
737737

738738
PJ_ASSERT_RETURN(buf && rpsi, PJ_EINVAL);
739-
PJ_ASSERT_RETURN(length >= sizeof(pjmedia_rtcp_common), PJ_ETOOSMALL);
739+
PJ_ASSERT_RETURN(length >= sizeof(pjmedia_rtcp_fb_common), PJ_ETOOSMALL);
740740

741741
/* RPSI uses pt==RTCP_PSFB and FMT==3 */
742-
if (hdr->pt != RTCP_PSFB || hdr->count != 3)
742+
if (hdr->rtcp_common.pt != RTCP_PSFB || hdr->rtcp_common.count != 3)
743743
return PJ_ENOTFOUND;
744744

745-
rpsi_len = (pj_ntohs((pj_uint16_t)hdr->length)-2) * 4;
745+
if (hdr->rtcp_common.length < 3) {
746+
PJ_PERROR(3, (THIS_FILE, PJ_ETOOSMALL,
747+
"Failed parsing FB RPSI, invalid header length"));
748+
return PJ_ETOOSMALL;
749+
}
750+
751+
rpsi_len = (pj_ntohs((pj_uint16_t)hdr->rtcp_common.length)-2) * 4;
746752
if (length < rpsi_len + 12)
747753
return PJ_ETOOSMALL;
748754

749755
p = (pj_uint8_t*)hdr + sizeof(*hdr);
750756
padlen = *p++;
757+
758+
if (padlen >= 32) {
759+
PJ_PERROR(3, (THIS_FILE, PJ_ETOOBIG,
760+
"Failed parsing FB RPSI, invalid RPSI padding len"));
761+
return PJ_ETOOBIG;
762+
}
763+
764+
if ((rpsi_len * 8) < (unsigned)(16 + padlen)) {
765+
PJ_PERROR(3, (THIS_FILE, PJ_ETOOSMALL,
766+
"Failed parsing FB RPSI, invalid RPSI bit len"));
767+
return PJ_ETOOSMALL;
768+
}
769+
751770
rpsi->pt = (*p++ & 0x7F);
752771
rpsi->rpsi_bit_len = rpsi_len*8 - 16 - padlen;
753772
pj_strset(&rpsi->rpsi, (char*)p, (rpsi->rpsi_bit_len + 7)/8);

0 commit comments

Comments
 (0)