From c2a5b88275445e593dcdc945bb2dfbe2a8118efc Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 3 Feb 2021 17:20:09 +1100 Subject: [PATCH] Attempt to silence untrusted loop bound Assign hit_len + key_len to len and test the result rather than recomputing and letting the compiler simplify. 213 isc_region_consume(®ion, 2); /* hit length + algorithm */ 9. tainted_return_value: Function uint16_fromregion returns tainted data. [show details] 10. tainted_data_transitive: Call to function uint16_fromregion with tainted argument *region.base returns tainted data. 11. tainted_return_value: Function uint16_fromregion returns tainted data. 12. tainted_data_transitive: Call to function uint16_fromregion with tainted argument *region.base returns tainted data. 13. var_assign: Assigning: key_len = uint16_fromregion(®ion), which taints key_len. 214 key_len = uint16_fromregion(®ion); 14. lower_bounds: Casting narrower unsigned key_len to wider signed type int effectively tests its lower bound. 15. Condition key_len == 0, taking false branch. 215 if (key_len == 0) { 216 RETERR(DNS_R_FORMERR); 217 } 16. Condition !!(_r->length >= _l), taking true branch. 17. Condition !!(_r->length >= _l), taking true branch. 218 isc_region_consume(®ion, 2); 18. lower_bounds: Casting narrower unsigned key_len to wider signed type int effectively tests its lower bound. 19. Condition region.length < (unsigned int)(hit_len + key_len), taking false branch. 219 if (region.length < (unsigned)(hit_len + key_len)) { 220 RETERR(DNS_R_FORMERR); 221 } 222 20. lower_bounds: Casting narrower unsigned key_len to wider signed type int effectively tests its lower bound. 21. Condition _r != 0, taking false branch. 223 RETERR(mem_tobuffer(target, rr.base, 4 + hit_len + key_len)); 22. lower_bounds: Casting narrower unsigned key_len to wider signed type int effectively tests its lower bound. 23. var_assign_var: Compound assignment involving tainted variable 4 + hit_len + key_len to variable source->current taints source->current. 224 isc_buffer_forward(source, 4 + hit_len + key_len); 225 226 dns_decompress_setmethods(dctx, DNS_COMPRESS_NONE); CID 281461 (#1 of 1): Untrusted loop bound (TAINTED_SCALAR) 24. tainted_data: Using tainted variable source->active - source->current as a loop boundary. Ensure that tainted values are properly sanitized, by checking that their values are within a permissible range. 227 while (isc_buffer_activelength(source) > 0) { 228 dns_name_init(&name, NULL); 229 RETERR(dns_name_fromwire(&name, source, dctx, options, target)); 230 } (cherry picked from commit 2f946c831a5ad4ed5c2694c51531dd4efa3b7aed) --- lib/dns/rdata/generic/hip_55.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/dns/rdata/generic/hip_55.c b/lib/dns/rdata/generic/hip_55.c index 772347ab156..9ead94ed4bb 100644 --- a/lib/dns/rdata/generic/hip_55.c +++ b/lib/dns/rdata/generic/hip_55.c @@ -194,6 +194,7 @@ fromwire_hip(ARGS_FROMWIRE) { dns_name_t name; uint8_t hit_len; uint16_t key_len; + size_t len; REQUIRE(type == dns_rdatatype_hip); @@ -216,12 +217,13 @@ fromwire_hip(ARGS_FROMWIRE) { RETERR(DNS_R_FORMERR); } isc_region_consume(®ion, 2); - if (region.length < (unsigned)(hit_len + key_len)) { + len = hit_len + key_len; + if (len > region.length) { RETERR(DNS_R_FORMERR); } - RETERR(mem_tobuffer(target, rr.base, 4 + hit_len + key_len)); - isc_buffer_forward(source, 4 + hit_len + key_len); + RETERR(mem_tobuffer(target, rr.base, 4 + len)); + isc_buffer_forward(source, 4 + len); dns_decompress_setmethods(dctx, DNS_COMPRESS_NONE); while (isc_buffer_activelength(source) > 0) { -- 2.47.3