gvsafronov/futriix
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

Fix invalid access in lpFind on corrupted listpack (#9819)

Browse Source 
Issue found by corrupt-dump-fuzzer test with ASAN.
The problem was that lpSkip and lpGetWithSize could read the next listpack entry without validating that it's in range.
Similarly even the memcmp in lpFind could do that and possibly crash on segfault and now they'll crash on assert first.

The naive fix of using lpAssertValidEntry every time, resulted in 30% degradation in the lpFind benchmark of the unit test.
The final fix with the condition at the bottom has no performance implications.
This commit is contained in:
Oran Agra 2021-11-22 15:30:00 +02:00 committed by GitHub
parent f00a8ad93c
commit f07dedf73f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 18 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
src/listpack.c
View File

@ -699,6 +699,8 @@ unsigned char *lpFind(unsigned char *lp, unsigned char *p, unsigned char *s,
if (skipcnt == 0) {
value = lpGetWithSize(p, &ll, NULL, &entry_size);
if (value) {
/* check the value doesn't reach outside the listpack before accessing it */
assert(p >= lp + LP_HDR_SIZE && p + entry_size < lp + lp_bytes);
if (slen == ll && memcmp(value, s, slen) == 0) {
return p;
}
@ -737,7 +739,12 @@ unsigned char *lpFind(unsigned char *lp, unsigned char *p, unsigned char *s,
p = lpSkip(p);
}
assert(p >= lp + LP_HDR_SIZE && p < lp + lp_bytes);
/* The next call to lpGetWithSize could read at most 8 bytes past `p`
* We use the slower validation call only when necessary. */
if (p + 8 >= lp + lp_bytes)
lpAssertValidEntry(lp, lp_bytes, p);
else
assert(p >= lp + LP_HDR_SIZE && p < lp + lp_bytes);
if (p[0] == LP_EOF) break;
}

10
tests/integration/corrupt-dump.tcl
View File

@ -764,5 +764,15 @@ test {corrupt payload: fuzzer findings - gcc asan reports false leak on assert}
}
}
test {corrupt payload: fuzzer findings - lpFind invalid access } {
start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] {
r debug set-skip-checksum-validation 1
r config set sanitize-dump-payload no
r restore _hashbig 0 "\x10\x39\x39\x00\x00\x00\x14\x00\x06\x01\x06\x01\x03\x01\x82\x5F\x33\x03\x07\x01\x82\x5F\x37\x03\x00\x01\x00\x01\x04\x01\x04\x01\x09\x01\x82\x5F\x39\x03\x05\x01\x82\x5F\x35\x03\x08\x01\x08\x01\x01\x01\x82\x5F\x31\x03\x02\x01\xF0\x01\xFF\x0A\x00\x29\xD7\xE4\x52\x79\x7A\x95\x82"
catch { r HLEN _hashbig }
catch { r HSETNX _hashbig 513072881620 "\x9A\x4B\x1F\xF2\x99\x74\x6E\x96\x84\x7F\xB9\x85\xBE\xD6\x1A\x93\x0A\xED\xAE\x19\xA0\x5A\x67\xD6\x89\xA8\xF9\xF2\xB8\xBD\x3E\x5A\xCF\xD2\x5B\x17\xA4\xBB\xB2\xA9\x56\x67\x6E\x0B\xED\xCD\x36\x49\xC6\x84\xFF\xC2\x76\x9B\xF3\x49\x88\x97\x92\xD2\x54\xE9\x08\x19\x86\x40\x96\x24\x68\x25\x9D\xF7\x0E\xB7\x36\x85\x68\x6B\x2A\x97\x64\x30\xE6\xFF\x9A\x2A\x42\x2B\x31\x01\x32\xB3\xEE\x78\x1A\x26\x94\xE2\x07\x34\x50\x8A\xFF\xF9\xAE\xEA\xEC\x59\x42\xF5\x39\x40\x65\xDE\x55\xCC\x77\x1B\x32\x02\x19\xEE\x3C\xD4\x79\x48\x01\x4F\x51\xFE\x22\xE0\x0C\xF4\x07\x06\xCD\x55\x30\xC0\x24\x32\xD4\xCC\xAF\x82\x05\x48\x14\x10\x55\xA1\x3D\xF6\x81\x45\x54\xEA\x71\x24\x27\x06\xDC\xFA\xE4\xE4\x87\xCC\x81\xA0\x47\xA5\xAF\xD1\x89\xE7\x42\xC3\x24\xD0\x32\x7A\xDE\x44\x47\x6E\x1F\xCB\xEE\xA6\x46\xDE\x0D\xE6\xD5\x16\x03\x2A\xD6\x9E\xFD\x94\x02\x2C\xDB\x1F\xD0\xBE\x98\x10\xE3\xEB\xEA\xBE\xE5\xD1" }
}
}
} ;# tags