mirror of
http://github.com/valkey-io/valkey
synced 2024-11-22 00:52:38 +00:00
On 32 bit platform, the bit position of GETBIT/SETBIT/BITFIELD/BITCOUNT,BITPOS may overflow (see CVE-2021-32761) (#9191)
GETBIT, SETBIT may access wrong address because of wrap. BITCOUNT and BITPOS may return wrapped results. BITFIELD may access the wrong address but also allocate insufficient memory and segfault (see CVE-2021-32761). This commit uses `uint64_t` or `long long` instead of `size_t`. related https://github.com/redis/redis/pull/8096 At 32bit platform: > setbit bit 4294967295 1 (integer) 0 > config set proto-max-bulk-len 536870913 OK > append bit "\xFF" (integer) 536870913 > getbit bit 4294967296 (integer) 0 When the bit index is larger than 4294967295, size_t can't hold bit index. In the past, `proto-max-bulk-len` is limit to 536870912, so there is no problem. After this commit, bit position is stored in `uint64_t` or `long long`. So when `proto-max-bulk-len > 536870912`, 32bit platforms can still be correct. For 64bit platform, this problem still exists. The major reason is bit pos 8 times of byte pos. When proto-max-bulk-len is very larger, bit pos may overflow. But at 64bit platform, we don't have so long string. So this bug may never happen. Additionally this commit add a test cost `512MB` memory which is tag as `large-memory`. Make freebsd ci and valgrind ci ignore this test.
This commit is contained in:
parent
32e61ee295
commit
71d452876e
6
.github/workflows/daily.yml
vendored
6
.github/workflows/daily.yml
vendored
@ -254,7 +254,7 @@ jobs:
|
||||
sudo apt-get install tcl8.6 tclx valgrind -y
|
||||
- name: test
|
||||
if: true && !contains(github.event.inputs.skiptests, 'redis')
|
||||
run: ./runtest --valgrind --verbose --clients 1 --dump-logs ${{github.event.inputs.test_args}}
|
||||
run: ./runtest --valgrind --verbose --clients 1 --tags -large-memory --dump-logs ${{github.event.inputs.test_args}}
|
||||
- name: module api test
|
||||
if: true && !contains(github.event.inputs.skiptests, 'modules')
|
||||
run: ./runtest-moduleapi --valgrind --no-latency --verbose --clients 1 ${{github.event.inputs.test_args}}
|
||||
@ -285,7 +285,7 @@ jobs:
|
||||
sudo apt-get install tcl8.6 tclx valgrind -y
|
||||
- name: test
|
||||
if: true && !contains(github.event.inputs.skiptests, 'redis')
|
||||
run: ./runtest --valgrind --verbose --clients 1 --dump-logs ${{github.event.inputs.test_args}}
|
||||
run: ./runtest --valgrind --verbose --clients 1 --tags -large-memory --dump-logs ${{github.event.inputs.test_args}}
|
||||
- name: module api test
|
||||
if: true && !contains(github.event.inputs.skiptests, 'modules')
|
||||
run: ./runtest-moduleapi --valgrind --no-latency --verbose --clients 1 ${{github.event.inputs.test_args}}
|
||||
@ -420,7 +420,7 @@ jobs:
|
||||
prepare: pkg install -y bash gmake lang/tcl86 lang/tclx
|
||||
run: >
|
||||
gmake || exit 1 ;
|
||||
if echo "${{github.event.inputs.skiptests}}" | grep -vq redis ; then ./runtest --accurate --verbose --no-latency --dump-logs ${{github.event.inputs.test_args}} || exit 1 ; fi ;
|
||||
if echo "${{github.event.inputs.skiptests}}" | grep -vq redis ; then ./runtest --accurate --verbose --no-latency --tags -large-memory --dump-logs ${{github.event.inputs.test_args}} || exit 1 ; fi ;
|
||||
if echo "${{github.event.inputs.skiptests}}" | grep -vq modules ; then MAKE=gmake ./runtest-moduleapi --verbose ${{github.event.inputs.test_args}} || exit 1 ; fi ;
|
||||
if echo "${{github.event.inputs.skiptests}}" | grep -vq sentinel ; then ./runtest-sentinel ${{github.event.inputs.cluster_test_args}} || exit 1 ; fi ;
|
||||
if echo "${{github.event.inputs.skiptests}}" | grep -vq cluster ; then ./runtest-cluster ${{github.event.inputs.cluster_test_args}} || exit 1 ; fi ;
|
||||
|
32
src/bitops.c
32
src/bitops.c
@ -37,8 +37,8 @@
|
||||
/* Count number of bits set in the binary array pointed by 's' and long
|
||||
* 'count' bytes. The implementation of this function is required to
|
||||
* work with an input string length up to 512 MB or more (server.proto_max_bulk_len) */
|
||||
size_t redisPopcount(void *s, long count) {
|
||||
size_t bits = 0;
|
||||
long long redisPopcount(void *s, long count) {
|
||||
long long bits = 0;
|
||||
unsigned char *p = s;
|
||||
uint32_t *p4;
|
||||
static const unsigned char bitsinbyte[256] = {0,1,1,2,1,2,2,3,1,2,2,3,2,3,3,4,1,2,2,3,2,3,3,4,2,3,3,4,3,4,4,5,1,2,2,3,2,3,3,4,2,3,3,4,3,4,4,5,2,3,3,4,3,4,4,5,3,4,4,5,4,5,5,6,1,2,2,3,2,3,3,4,2,3,3,4,3,4,4,5,2,3,3,4,3,4,4,5,3,4,4,5,4,5,5,6,2,3,3,4,3,4,4,5,3,4,4,5,4,5,5,6,3,4,4,5,4,5,5,6,4,5,5,6,5,6,6,7,1,2,2,3,2,3,3,4,2,3,3,4,3,4,4,5,2,3,3,4,3,4,4,5,3,4,4,5,4,5,5,6,2,3,3,4,3,4,4,5,3,4,4,5,4,5,5,6,3,4,4,5,4,5,5,6,4,5,5,6,5,6,6,7,2,3,3,4,3,4,4,5,3,4,4,5,4,5,5,6,3,4,4,5,4,5,5,6,4,5,5,6,5,6,6,7,3,4,4,5,4,5,5,6,4,5,5,6,5,6,6,7,4,5,5,6,5,6,6,7,5,6,6,7,6,7,7,8};
|
||||
@ -98,11 +98,11 @@ size_t redisPopcount(void *s, long count) {
|
||||
* no zero bit is found, it returns count*8 assuming the string is zero
|
||||
* padded on the right. However if 'bit' is 1 it is possible that there is
|
||||
* not a single set bit in the bitmap. In this special case -1 is returned. */
|
||||
long redisBitpos(void *s, unsigned long count, int bit) {
|
||||
long long redisBitpos(void *s, unsigned long count, int bit) {
|
||||
unsigned long *l;
|
||||
unsigned char *c;
|
||||
unsigned long skipval, word = 0, one;
|
||||
long pos = 0; /* Position of bit, to return to the caller. */
|
||||
long long pos = 0; /* Position of bit, to return to the caller. */
|
||||
unsigned long j;
|
||||
int found;
|
||||
|
||||
@ -410,7 +410,7 @@ void printBits(unsigned char *p, unsigned long count) {
|
||||
* If the 'hash' argument is true, and 'bits is positive, then the command
|
||||
* will also parse bit offsets prefixed by "#". In such a case the offset
|
||||
* is multiplied by 'bits'. This is useful for the BITFIELD command. */
|
||||
int getBitOffsetFromArgument(client *c, robj *o, size_t *offset, int hash, int bits) {
|
||||
int getBitOffsetFromArgument(client *c, robj *o, uint64_t *offset, int hash, int bits) {
|
||||
long long loffset;
|
||||
char *err = "bit offset is not an integer or out of range";
|
||||
char *p = o->ptr;
|
||||
@ -435,7 +435,7 @@ int getBitOffsetFromArgument(client *c, robj *o, size_t *offset, int hash, int b
|
||||
return C_ERR;
|
||||
}
|
||||
|
||||
*offset = (size_t)loffset;
|
||||
*offset = loffset;
|
||||
return C_OK;
|
||||
}
|
||||
|
||||
@ -477,7 +477,7 @@ int getBitfieldTypeFromArgument(client *c, robj *o, int *sign, int *bits) {
|
||||
* so that the 'maxbit' bit can be addressed. The object is finally
|
||||
* returned. Otherwise if the key holds a wrong type NULL is returned and
|
||||
* an error is sent to the client. */
|
||||
robj *lookupStringForBitCommand(client *c, size_t maxbit) {
|
||||
robj *lookupStringForBitCommand(client *c, uint64_t maxbit) {
|
||||
size_t byte = maxbit >> 3;
|
||||
robj *o = lookupKeyWrite(c->db,c->argv[1]);
|
||||
if (checkType(c,o,OBJ_STRING)) return NULL;
|
||||
@ -527,7 +527,7 @@ unsigned char *getObjectReadOnlyString(robj *o, long *len, char *llbuf) {
|
||||
void setbitCommand(client *c) {
|
||||
robj *o;
|
||||
char *err = "bit is not an integer or out of range";
|
||||
size_t bitoffset;
|
||||
uint64_t bitoffset;
|
||||
ssize_t byte, bit;
|
||||
int byteval, bitval;
|
||||
long on;
|
||||
@ -566,7 +566,7 @@ void setbitCommand(client *c) {
|
||||
void getbitCommand(client *c) {
|
||||
robj *o;
|
||||
char llbuf[32];
|
||||
size_t bitoffset;
|
||||
uint64_t bitoffset;
|
||||
size_t byte, bit;
|
||||
size_t bitval = 0;
|
||||
|
||||
@ -888,7 +888,7 @@ void bitposCommand(client *c) {
|
||||
addReplyLongLong(c, -1);
|
||||
} else {
|
||||
long bytes = end-start+1;
|
||||
long pos = redisBitpos(p+start,bytes,bit);
|
||||
long long pos = redisBitpos(p+start,bytes,bit);
|
||||
|
||||
/* If we are looking for clear bits, and the user specified an exact
|
||||
* range with start-end, we can't consider the right of the range as
|
||||
@ -897,11 +897,11 @@ void bitposCommand(client *c) {
|
||||
* So if redisBitpos() returns the first bit outside the range,
|
||||
* we return -1 to the caller, to mean, in the specified range there
|
||||
* is not a single "0" bit. */
|
||||
if (end_given && bit == 0 && pos == bytes*8) {
|
||||
if (end_given && bit == 0 && pos == (long long)bytes<<3) {
|
||||
addReplyLongLong(c,-1);
|
||||
return;
|
||||
}
|
||||
if (pos != -1) pos += start*8; /* Adjust for the bytes we skipped. */
|
||||
if (pos != -1) pos += (long long)start<<3; /* Adjust for the bytes we skipped. */
|
||||
addReplyLongLong(c,pos);
|
||||
}
|
||||
}
|
||||
@ -933,12 +933,12 @@ struct bitfieldOp {
|
||||
* GET subcommand is allowed, other subcommands will return an error. */
|
||||
void bitfieldGeneric(client *c, int flags) {
|
||||
robj *o;
|
||||
size_t bitoffset;
|
||||
uint64_t bitoffset;
|
||||
int j, numops = 0, changes = 0;
|
||||
struct bitfieldOp *ops = NULL; /* Array of ops to execute at end. */
|
||||
int owtype = BFOVERFLOW_WRAP; /* Overflow type. */
|
||||
int readonly = 1;
|
||||
size_t highest_write_offset = 0;
|
||||
uint64_t highest_write_offset = 0;
|
||||
|
||||
for (j = 2; j < c->argc; j++) {
|
||||
int remargs = c->argc-j-1; /* Remaining args other than current. */
|
||||
@ -1128,9 +1128,9 @@ void bitfieldGeneric(client *c, int flags) {
|
||||
* object boundaries. */
|
||||
memset(buf,0,9);
|
||||
int i;
|
||||
size_t byte = thisop->offset >> 3;
|
||||
uint64_t byte = thisop->offset >> 3;
|
||||
for (i = 0; i < 9; i++) {
|
||||
if (src == NULL || i+byte >= (size_t)strlen) break;
|
||||
if (src == NULL || i+byte >= (uint64_t)strlen) break;
|
||||
buf[i] = src[i+byte];
|
||||
}
|
||||
|
||||
|
@ -1845,7 +1845,7 @@ void getRandomHexChars(char *p, size_t len);
|
||||
void getRandomBytes(unsigned char *p, size_t len);
|
||||
uint64_t crc64(uint64_t crc, const unsigned char *s, uint64_t l);
|
||||
void exitFromChild(int retcode);
|
||||
size_t redisPopcount(void *s, long count);
|
||||
long long redisPopcount(void *s, long count);
|
||||
int redisSetProcTitle(char *title);
|
||||
int validateProcTitleTemplate(const char *template);
|
||||
int redisCommunicateSystemd(const char *sd_notify_msg);
|
||||
|
@ -348,4 +348,30 @@ start_server {tags {"bitops"}} {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
test "BIT pos larger than UINT_MAX" {
|
||||
set bytes [expr (1 << 29) + 1]
|
||||
set bitpos [expr (1 << 32)]
|
||||
set oldval [lindex [r config get proto-max-bulk-len] 1]
|
||||
r config set proto-max-bulk-len $bytes
|
||||
r setbit mykey $bitpos 1
|
||||
assert_equal $bytes [r strlen mykey]
|
||||
assert_equal 1 [r getbit mykey $bitpos]
|
||||
assert_equal [list 128 128 -1] [r bitfield mykey get u8 $bitpos set u8 $bitpos 255 get i8 $bitpos]
|
||||
assert_equal $bitpos [r bitpos mykey 1]
|
||||
assert_equal $bitpos [r bitpos mykey 1 [expr $bytes - 1]]
|
||||
if {$::accurate} {
|
||||
# set all bits to 1
|
||||
set mega [expr (1 << 23)]
|
||||
set part [string repeat "\xFF" $mega]
|
||||
for {set i 0} {$i < 64} {incr i} {
|
||||
r setrange mykey [expr $i * $mega] $part
|
||||
}
|
||||
r setrange mykey [expr $bytes - 1] "\xFF"
|
||||
assert_equal [expr $bitpos + 8] [r bitcount mykey]
|
||||
assert_equal -1 [r bitpos mykey 0 0 [expr $bytes - 1]]
|
||||
}
|
||||
r config set proto-max-bulk-len $oldval
|
||||
r del mykey
|
||||
} {1} {large-memory}
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user