Commit 449af2cd authored by Huang Zhw's avatar Huang Zhw Committed by Oran Agra
Browse files

On 32 bit platform, the bit position of GETBIT/SETBIT/BITFIELD/BITCOUNT,BITPOS...

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 test is disabled in this version since bitops doesn't rely on
proto-max-bulk-len. some of the overflows can still occur so we do want
the fixes.

(cherry picked from commit 71d45287)
(cherry picked from commit 2be8f1de1d452886fbf45030409707c52323da20)
parent 0dfc71bd
...@@ -37,8 +37,8 @@ ...@@ -37,8 +37,8 @@
/* Count number of bits set in the binary array pointed by 's' and long /* Count number of bits set in the binary array pointed by 's' and long
* 'count' bytes. The implementation of this function is required to * 'count' bytes. The implementation of this function is required to
* work with a input string length up to 512 MB. */ * work with a input string length up to 512 MB. */
size_t redisPopcount(void *s, long count) { long long redisPopcount(void *s, long count) {
size_t bits = 0; long long bits = 0;
unsigned char *p = s; unsigned char *p = s;
uint32_t *p4; 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}; 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) { ...@@ -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 * 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 * 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. */ * 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 long *l;
unsigned char *c; unsigned char *c;
unsigned long skipval, word = 0, one; 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; unsigned long j;
int found; int found;
...@@ -408,7 +408,7 @@ void printBits(unsigned char *p, unsigned long count) { ...@@ -408,7 +408,7 @@ void printBits(unsigned char *p, unsigned long count) {
* If the 'hash' argument is true, and 'bits is positive, then the command * 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 * will also parse bit offsets prefixed by "#". In such a case the offset
* is multiplied by 'bits'. This is useful for the BITFIELD command. */ * 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; long long loffset;
char *err = "bit offset is not an integer or out of range"; char *err = "bit offset is not an integer or out of range";
char *p = o->ptr; char *p = o->ptr;
...@@ -433,7 +433,7 @@ int getBitOffsetFromArgument(client *c, robj *o, size_t *offset, int hash, int b ...@@ -433,7 +433,7 @@ int getBitOffsetFromArgument(client *c, robj *o, size_t *offset, int hash, int b
return C_ERR; return C_ERR;
} }
*offset = (size_t)loffset; *offset = loffset;
return C_OK; return C_OK;
} }
...@@ -475,7 +475,7 @@ int getBitfieldTypeFromArgument(client *c, robj *o, int *sign, int *bits) { ...@@ -475,7 +475,7 @@ int getBitfieldTypeFromArgument(client *c, robj *o, int *sign, int *bits) {
* so that the 'maxbit' bit can be addressed. The object is finally * 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 * returned. Otherwise if the key holds a wrong type NULL is returned and
* an error is sent to the client. */ * 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; size_t byte = maxbit >> 3;
robj *o = lookupKeyWrite(c->db,c->argv[1]); robj *o = lookupKeyWrite(c->db,c->argv[1]);
...@@ -525,7 +525,7 @@ unsigned char *getObjectReadOnlyString(robj *o, long *len, char *llbuf) { ...@@ -525,7 +525,7 @@ unsigned char *getObjectReadOnlyString(robj *o, long *len, char *llbuf) {
void setbitCommand(client *c) { void setbitCommand(client *c) {
robj *o; robj *o;
char *err = "bit is not an integer or out of range"; char *err = "bit is not an integer or out of range";
size_t bitoffset; uint64_t bitoffset;
ssize_t byte, bit; ssize_t byte, bit;
int byteval, bitval; int byteval, bitval;
long on; long on;
...@@ -564,7 +564,7 @@ void setbitCommand(client *c) { ...@@ -564,7 +564,7 @@ void setbitCommand(client *c) {
void getbitCommand(client *c) { void getbitCommand(client *c) {
robj *o; robj *o;
char llbuf[32]; char llbuf[32];
size_t bitoffset; uint64_t bitoffset;
size_t byte, bit; size_t byte, bit;
size_t bitval = 0; size_t bitval = 0;
...@@ -874,7 +874,7 @@ void bitposCommand(client *c) { ...@@ -874,7 +874,7 @@ void bitposCommand(client *c) {
addReplyLongLong(c, -1); addReplyLongLong(c, -1);
} else { } else {
long bytes = end-start+1; 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 /* 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 * range with start-end, we can't consider the right of the range as
...@@ -883,11 +883,11 @@ void bitposCommand(client *c) { ...@@ -883,11 +883,11 @@ void bitposCommand(client *c) {
* So if redisBitpos() returns the first bit outside the range, * So if redisBitpos() returns the first bit outside the range,
* we return -1 to the caller, to mean, in the specified range there * we return -1 to the caller, to mean, in the specified range there
* is not a single "0" bit. */ * 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); addReplyLongLong(c,-1);
return; 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); addReplyLongLong(c,pos);
} }
} }
...@@ -913,12 +913,12 @@ struct bitfieldOp { ...@@ -913,12 +913,12 @@ struct bitfieldOp {
void bitfieldCommand(client *c) { void bitfieldCommand(client *c) {
robj *o; robj *o;
size_t bitoffset; uint64_t bitoffset;
int j, numops = 0, changes = 0; int j, numops = 0, changes = 0;
struct bitfieldOp *ops = NULL; /* Array of ops to execute at end. */ struct bitfieldOp *ops = NULL; /* Array of ops to execute at end. */
int owtype = BFOVERFLOW_WRAP; /* Overflow type. */ int owtype = BFOVERFLOW_WRAP; /* Overflow type. */
int readonly = 1; int readonly = 1;
size_t highest_write_offset = 0; uint64_t highest_write_offset = 0;
for (j = 2; j < c->argc; j++) { for (j = 2; j < c->argc; j++) {
int remargs = c->argc-j-1; /* Remaining args other than current. */ int remargs = c->argc-j-1; /* Remaining args other than current. */
...@@ -1102,9 +1102,9 @@ void bitfieldCommand(client *c) { ...@@ -1102,9 +1102,9 @@ void bitfieldCommand(client *c) {
* object boundaries. */ * object boundaries. */
memset(buf,0,9); memset(buf,0,9);
int i; int i;
size_t byte = thisop->offset >> 3; uint64_t byte = thisop->offset >> 3;
for (i = 0; i < 9; i++) { 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]; buf[i] = src[i+byte];
} }
......
...@@ -1436,7 +1436,7 @@ void getRandomHexChars(char *p, size_t len); ...@@ -1436,7 +1436,7 @@ void getRandomHexChars(char *p, size_t len);
void getRandomBytes(unsigned 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); uint64_t crc64(uint64_t crc, const unsigned char *s, uint64_t l);
void exitFromChild(int retcode); void exitFromChild(int retcode);
size_t redisPopcount(void *s, long count); long long redisPopcount(void *s, long count);
void redisSetProcTitle(char *title); void redisSetProcTitle(char *title);
/* networking.c -- Networking and Client related operations */ /* networking.c -- Networking and Client related operations */
......
...@@ -349,3 +349,34 @@ start_server {tags {"bitops"}} { ...@@ -349,3 +349,34 @@ start_server {tags {"bitops"}} {
} }
} }
} }
# test disabled because in this version bitops doesn't use proto-max-bulk-len
if {0} {
start_server {tags {"bitops large-memory"}} {
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}
}
}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment