Commit 052e01e7 authored by Ozan Tezcan's avatar Ozan Tezcan Committed by Oran Agra
Browse files

Some fixes to undefined behaviour bugs taken from (#9601)

**Signed integer overflow.** Although, signed overflow issue can be problematic time to time
and change how compiler generates code, current findings mostly about signed shift or simple
addition overflow. For most platforms Redis can be compiled for, this wouldn't cause any issue
as far as I can tell (checked generated code on godbolt.org).

UB means nothing guaranteed and risky to reason about program behavior but I don't think any
of the fixes here worth backporting. As sanitizers are now part of the CI, preventing new issues
will be the real benefit.

partial cherry pick from commit b91d8b28
The bug in BITFIELD seems to affect 12.2.1 used on Alpine
parent 5d66aa3d
......@@ -328,8 +328,9 @@ int checkSignedBitfieldOverflow(int64_t value, int64_t incr, uint64_t bits, int
/* Note that maxincr and minincr could overflow, but we use the values
* only after checking 'value' range, so when we use it no overflow
* happens. */
int64_t maxincr = max-value;
* happens. 'uint64_t' cast is there just to prevent undefined behavior on
* overflow */
int64_t maxincr = (uint64_t)max-value;
int64_t minincr = min-value;
if (value > max || (bits != 64 && incr > maxincr) || (value >= 0 && incr > 0 && incr > maxincr))
......
......@@ -388,6 +388,7 @@ void initConfigValues() {
}
void loadServerConfigFromString(char *config) {
char buf[1024];
const char *err = NULL;
int linenum = 0, totlines, i;
int slaveof_linenum = 0;
......@@ -588,7 +589,6 @@ void loadServerConfigFromString(char *config) {
} else if (!strcasecmp(argv[0],"user") && argc >= 2) {
int argc_err;
if (ACLAppendUserForLoading(argv,argc,&argc_err) == C_ERR) {
char buf[1024];
const char *errmsg = ACLSetUserStringError();
snprintf(buf,sizeof(buf),"Error in user declaration '%s': %s",
argv[argc_err],errmsg);
......
......@@ -534,8 +534,8 @@ void *dictFetchValue(dict *d, const void *key) {
* the fingerprint again when the iterator is released.
* If the two fingerprints are different it means that the user of the iterator
* performed forbidden operations against the dictionary while iterating. */
long long dictFingerprint(dict *d) {
long long integers[6], hash = 0;
unsigned long long dictFingerprint(dict *d) {
unsigned long long integers[6], hash = 0;
int j;
integers[0] = (long) d->ht[0].table;
......
......@@ -95,7 +95,7 @@ typedef struct dictIterator {
int table, safe;
dictEntry *entry, *nextEntry;
/* unsafe iterator fingerprint for misuse detection. */
long long fingerprint;
unsigned long long fingerprint;
} dictIterator;
typedef void (dictScanFunction)(void *privdata, const dictEntry *de);
......
......@@ -1270,6 +1270,7 @@ void raxStart(raxIterator *it, rax *rt) {
* is a low level function used to implement the iterator, not callable by
* the user. Returns 0 on out of memory, otherwise 1 is returned. */
int raxIteratorAddChars(raxIterator *it, unsigned char *s, size_t len) {
if (len == 0) return 1;
if (it->key_max < it->key_len+len) {
unsigned char *old = (it->key == it->key_static_string) ? NULL :
it->key;
......
......@@ -288,12 +288,16 @@ void *rdbLoadIntegerObject(rio *rdb, int enctype, int flags, size_t *lenptr) {
} else if (enctype == RDB_ENC_INT16) {
uint16_t v;
if (rioRead(rdb,enc,2) == 0) return NULL;
v = enc[0]|(enc[1]<<8);
v = ((uint32_t)enc[0])|
((uint32_t)enc[1]<<8);
val = (int16_t)v;
} else if (enctype == RDB_ENC_INT32) {
uint32_t v;
if (rioRead(rdb,enc,4) == 0) return NULL;
v = enc[0]|(enc[1]<<8)|(enc[2]<<16)|(enc[3]<<24);
v = ((uint32_t)enc[0])|
((uint32_t)enc[1]<<8)|
((uint32_t)enc[2]<<16)|
((uint32_t)enc[3]<<24);
val = (int32_t)v;
} else {
rdbReportCorruptRDB("Unknown RDB integer encoding type %d",enctype);
......
......@@ -45,8 +45,13 @@ void sha256_transform(SHA256_CTX *ctx, const BYTE data[])
{
WORD a, b, c, d, e, f, g, h, i, j, t1, t2, m[64];
for (i = 0, j = 0; i < 16; ++i, j += 4)
m[i] = (data[j] << 24) | (data[j + 1] << 16) | (data[j + 2] << 8) | (data[j + 3]);
for (i = 0, j = 0; i < 16; ++i, j += 4) {
m[i] = ((WORD) data[j + 0] << 24) |
((WORD) data[j + 1] << 16) |
((WORD) data[j + 2] << 8) |
((WORD) data[j + 3]);
}
for ( ; i < 64; ++i)
m[i] = SIG1(m[i - 2]) + m[i - 7] + SIG0(m[i - 15]) + m[i - 16];
......
......@@ -416,10 +416,10 @@ unsigned int zipStoreEntryEncoding(unsigned char *p, unsigned char encoding, uns
(len) = (((ptr)[0] & 0x3f) << 8) | (ptr)[1]; \
} else if ((encoding) == ZIP_STR_32B) { \
(lensize) = 5; \
(len) = ((ptr)[1] << 24) | \
((ptr)[2] << 16) | \
((ptr)[3] << 8) | \
((ptr)[4]); \
(len) = ((uint32_t)(ptr)[1] << 24) | \
((uint32_t)(ptr)[2] << 16) | \
((uint32_t)(ptr)[3] << 8) | \
((uint32_t)(ptr)[4]); \
} else { \
(lensize) = 0; /* bad encoding, should be covered by a previous */ \
(len) = 0; /* ZIP_ASSERT_ENCODING / zipEncodingLenSize, or */ \
......@@ -557,7 +557,7 @@ void zipSaveInteger(unsigned char *p, int64_t value, unsigned char encoding) {
memcpy(p,&i16,sizeof(i16));
memrev16ifbe(p);
} else if (encoding == ZIP_INT_24B) {
i32 = value<<8;
i32 = ((uint64_t)value)<<8;
memrev32ifbe(&i32);
memcpy(p,((uint8_t*)&i32)+1,sizeof(i32)-sizeof(uint8_t));
} else if (encoding == ZIP_INT_32B) {
......
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