Skip to content

Commit

Permalink
Merge pull request from GHSA-q9cp-8wcq-7pfr
Browse files Browse the repository at this point in the history 
* Prevent heap buffer overflow when parsing DNS packet

* Fixed incorrect check in get_name*()
  • Loading branch information
@sauwming
sauwming authored Mar 14, 2023
1 parent 5e2d564 commit d1c5e4d
Showing 1 changed file with 12 additions and 0 deletions.
12 changes: 12 additions & 0 deletions pjlib-util/src/pjlib-util/dns.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,9 @@ static pj_status_t get_name_len(int rec_counter, const pj_uint8_t *pkt,
return PJLIB_UTIL_EDNSINNAMEPTR;
}

if (start >= max)
return PJLIB_UTIL_EDNSINNAMEPTR;

*name_len = *parsed_len = 0;
p = start;
while (*p) {
Expand Down Expand Up @@ -199,6 +202,9 @@ static pj_status_t get_name(int rec_counter, const pj_uint8_t *pkt,
return PJLIB_UTIL_EDNSINNAMEPTR;
}

if (start >= max)
return PJLIB_UTIL_EDNSINNAMEPTR;

p = start;
while (*p) {
if ((*p & 0xc0) == 0xc0) {
Expand Down Expand Up @@ -359,10 +365,14 @@ static pj_status_t parse_rr(pj_dns_parsed_rr *rr, pj_pool_t *pool,

/* Parse some well known records */
if (rr->type == PJ_DNS_TYPE_A) {
if (p + 4 > max)
return PJLIB_UTIL_EDNSINSIZE;
pj_memcpy(&rr->rdata.a.ip_addr, p, 4);
p += 4;

} else if (rr->type == PJ_DNS_TYPE_AAAA) {
if (p + 16 > max)
return PJLIB_UTIL_EDNSINSIZE;
pj_memcpy(&rr->rdata.aaaa.ip_addr, p, 16);
p += 16;

Expand All @@ -388,6 +398,8 @@ static pj_status_t parse_rr(pj_dns_parsed_rr *rr, pj_pool_t *pool,
p += name_part_len;

} else if (rr->type == PJ_DNS_TYPE_SRV) {
if (p + 6 > max)
return PJLIB_UTIL_EDNSINSIZE;

/* Priority */
pj_memcpy(&rr->rdata.srv.prio, p, 2);
Expand Down

4 comments on commit d1c5e4d

@taladrane
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @sauwming from the GitHub Advisory Database and CVE team 👋 could you provide any more details about how GHSA-q9cp-8wcq-7pfr/CVE-2023-27585 is different than GHSA-p6g5-v97c-w5q4/CVE-2022-24793 since they are extremely similar? thanks!

@sauwming
Copy link
Member Author

@sauwming sauwming commented on d1c5e4d Mar 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, both are very similar since they both happen when parsing a DNS packet, and the problem was found using the same method, i.e. fuzzing.

We have fixed the first one in the first SA but as it turned out, there were still more issues found when using a different seed input.

The difference is that the first issue was in parsing the query record parse_query(), while the second is in parse_rr().

@taladrane
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for sharing this information with us @sauwming! we've updated the descriptions of the corresponding CVEs to better differentiate them for the rest of the community (CVE-2023-27585 PR & CVE-2022-24793 PR)

@sauwming
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I have updated our SA description as well to include the difference.

Please sign in to comment.