Skip to content

Commit fbdd35a

Browse files
committed
Address Copilot review on PR #3560
1. Add missing bound check before reading value_len (Copilot #1 — real bug). After consuming the name field, blob_offset can advance to exactly blob_size; the subsequent 16-bit read of value_len from blob[blob_offset] / blob[blob_offset+1] would then OOB-read on a truncated blob. Fixed with the standard 2-byte check. 2. Drop tests/regression/persist_dbm/ (Copilot #2-#5). The directory was not wired into the Autotools build (no AC_CONFIG_FILES nor parent SUBDIRS entry), and the existing tests/regression/ is a Perl-based HTTP integration harness that doesn't fit a unit test of a static function. Wiring it into tests/Makefile.am where msc_test lives would require non-trivial restructuring; keeping the standalone harness outside the upstream tree (in the security advisory's PoC archive) is the cleaner path for now. Refs: PR #3560 review comments by github-actions[bot] / Copilot.
1 parent d7759ab commit fbdd35a

3 files changed

Lines changed: 2 additions & 310 deletions

File tree

apache2/persist_dbm.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ static apr_table_t *collection_unpack(modsec_rec *msr, const unsigned char *blob
6161
}
6262

6363
blob_offset += 2;
64-
if (var->name_len < 1 || blob_offset + var->name_len > blob_size) return NULL;
64+
/* Need name_len bytes for the name body plus 2 more for the value_len header. */
65+
if (var->name_len < 1 || blob_offset + var->name_len + 2 > blob_size) return NULL;
6566
var->name = apr_pstrmemdup(msr->mp, (const char *)blob + blob_offset, var->name_len - 1);
6667
blob_offset += var->name_len;
6768
var->name_len--;

tests/regression/persist_dbm/Makefile.am

Lines changed: 0 additions & 5 deletions
This file was deleted.

tests/regression/persist_dbm/test_persist_dbm_value_len.c

Lines changed: 0 additions & 304 deletions
This file was deleted.

0 commit comments

Comments
 (0)