From 83fd38dd2ccf682fee117c389f17b2e0089e36aa Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 7 Jul 2021 12:09:31 +1000 Subject: [PATCH 1/2] Silence use of tainted scalar 2607 43. tainted_argument: Calling function journal_read_xhdr taints argument xhdr.size. [show details] 2608 result = journal_read_xhdr(j1, &xhdr); 44. Condition rewrite, taking true branch. 45. Condition result == 29, taking false branch. 2609 if (rewrite && result == ISC_R_NOMORE) { 2610 break; 2611 } 46. Condition result != 0, taking false branch. 2612 CHECK(result); 2613 47. var_assign_var: Assigning: size = xhdr.size. Both are now tainted. 2614 size = xhdr.size; CID 331088 (#3 of 3): Untrusted allocation size (TAINTED_SCALAR) 48. tainted_data: Passing tainted expression size to isc__mem_get, which uses it as an allocation size. [show details] Ensure that tainted values are properly sanitized, by checking that their values are within a permissible range. 2615 buf = isc_mem_get(mctx, size); --- lib/dns/journal.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/lib/dns/journal.c b/lib/dns/journal.c index 567692e7a4..f59855dd63 100644 --- a/lib/dns/journal.c +++ b/lib/dns/journal.c @@ -2613,6 +2613,14 @@ dns_journal_compact(isc_mem_t *mctx, char *filename, uint32_t serial, CHECK(result); size = xhdr.size; + if (size > len) { + isc_log_write(JOURNAL_COMMON_LOGARGS, + ISC_LOG_ERROR, + "%s: journal file corrupt, " + "transaction too large", + j1->filename); + CHECK(ISC_R_FAILURE); + } buf = isc_mem_get(mctx, size); result = journal_read(j1, buf, size); @@ -2637,6 +2645,15 @@ dns_journal_compact(isc_mem_t *mctx, char *filename, uint32_t serial, /* Check again */ isc_mem_put(mctx, buf, size); size = xhdr.size; + if (size > len) { + isc_log_write( + JOURNAL_COMMON_LOGARGS, + ISC_LOG_ERROR, + "%s: journal file corrupt, " + "transaction too large", + j1->filename); + CHECK(ISC_R_FAILURE); + } buf = isc_mem_get(mctx, size); CHECK(journal_read(j1, buf, size)); From f0fdca90f2baa3a8cc2e04fb47f367ef90486357 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 7 Jul 2021 14:08:04 +1000 Subject: [PATCH 2/2] Silence tainted scalar on rdlen 2042 ttl = isc_buffer_getuint32(&j->it.source); 13. tainted_data_transitive: Call to function isc_buffer_getuint16 with tainted argument *j->it.source.base returns tainted data. [show details] 14. var_assign: Assigning: rdlen = isc_buffer_getuint16(&j->it.source), which taints rdlen. 2043 rdlen = isc_buffer_getuint16(&j->it.source); 2044 2045 /* 2046 * Parse the rdata. 2047 */ 15. Condition j->it.source.used - j->it.source.current != rdlen, taking false branch. 2048 if (isc_buffer_remaininglength(&j->it.source) != rdlen) { 2049 FAIL(DNS_R_FORMERR); 2050 } 16. var_assign_var: Assigning: j->it.source.active = j->it.source.current + rdlen. Both are now tainted. 2051 isc_buffer_setactive(&j->it.source, rdlen); 2052 dns_rdata_reset(&j->it.rdata); 17. lower_bounds: Checking lower bounds of unsigned scalar j->it.source.active by taking the true branch of j->it.source.active > j->it.source.current. CID 316506 (#1 of 1): Untrusted loop bound (TAINTED_SCALAR) 18. tainted_data: Passing tainted expression j->it.source.active to dns_rdata_fromwire, which uses it as a loop boundary. [show details] Ensure that tainted values are properly sanitized, by checking that their values are within a permissible range. 2053 CHECK(dns_rdata_fromwire(&j->it.rdata, rdclass, rdtype, &j->it.source, 2054 &j->it.dctx, 0, &j->it.target)); --- lib/dns/journal.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/dns/journal.c b/lib/dns/journal.c index f59855dd63..48fbb433aa 100644 --- a/lib/dns/journal.c +++ b/lib/dns/journal.c @@ -2043,6 +2043,14 @@ read_one_rr(dns_journal_t *j) { ttl = isc_buffer_getuint32(&j->it.source); rdlen = isc_buffer_getuint16(&j->it.source); + if (rdlen > DNS_RDATA_MAXLENGTH) { + isc_log_write(JOURNAL_COMMON_LOGARGS, ISC_LOG_ERROR, + "%s: journal corrupt: impossible rdlen " + "(%u bytes)", + j->filename, rdlen); + FAIL(ISC_R_FAILURE); + } + /* * Parse the rdata. */