hiredis icon indicating copy to clipboard operation
hiredis copied to clipboard

Potential fault at createDoubleObject

Open afcidk opened this issue 3 years ago • 1 comments

The potential fault occurs at the parameter len of createDoubleObject.

From the function prototype, we can see that len is of size_t type, which is known as an unsigned integer. https://github.com/redis/hiredis/blob/b6f86f38c2bbf0caa63d489174ac3a9777b97807/hiredis.c#L221

This function attempts to allocate some space for string-typed double value dval, and allocates one more byte for the null-terminating character for the string.

https://github.com/redis/hiredis/blob/b6f86f38c2bbf0caa63d489174ac3a9777b97807/hiredis.c#L228-L233

When len is -1, the program allocates a space with size 0 for r->str. According to man 3 malloc,

If size is 0, then malloc() returns either NULL, or a unique pointer value that can later be successfully passed to free().

If hi_malloc does not return NULL, the program will crash in the following memcpy, which passes maximum value of unsigned integer to the size of memcpy.

https://github.com/redis/hiredis/blob/b6f86f38c2bbf0caa63d489174ac3a9777b97807/hiredis.c#L240

Additional check to len parameter should be included in createDoubleObject function.

afcidk avatar Jun 14 '21 02:06 afcidk

Why would you want to specify len as -1 (it is unsigned). Or do you mean to say that the largest possible len value causes a crash? Wouldn't it simply be better to specify the behaviour as undefined if len + 1 does not fit within a size_t ?

kristjanvalur avatar Aug 04 '21 12:08 kristjanvalur