sds icon indicating copy to clipboard operation
sds copied to clipboard

Null Dereferences v2.0.0

Open PatriciaSVMonteiro opened this issue 6 years ago • 2 comments

In many functions in file “sds.h”, the parameter “sds s” is dereferenced without checking if it is NULL. The same error is also present in some functions in file “sds.c”, such as: sdscat, sdsMakeRoomFor, sdsRemoveFreeSpace, sdsdup, sdsupdatelen, sdscatrepr, sdscmp, sdstoupper, sdstolower, sdsrange, sdstrim, sdscatfmt, sdsclear, sdslen, sdscatvprintf, sdscatprintf, sdscpy, sdscpylen, sdscatsds, sdscatlen, sdsgrowzero, sdsIncrLen, sdsAllocSize e sdsAllocPtr.

This functions should check for a parameter with value NULL and possibly return an error code in such case.

Minimal example:

int sdsTest(void) {
        sds x = NULL;
        test_cond("Create a string and obtain the length",
            sdslen(x) == 3 && memcmp(x,"foo\0",4) == 0)

    sdsfree(x);
    test_report();
    return 0;
}

Forcing the variable “sds s = NULL” while running the test programs generates a segmentation fault (due to the attempt to dereference NULL).

PatriciaSVMonteiro avatar May 15 '18 11:05 PatriciaSVMonteiro

This is not an error, even strlen() from string.h doesn't check for NULL. The rationale behind this design decision is that you cannot check the length of something that doesn't exist.

hu55a1n1 avatar Jun 02 '19 14:06 hu55a1n1

These functions don't check for NULL because s != NULL is a pre-condition; if, for example, sdslen receives NULL as an argument, I'm pretty sure that it's caller's fault.

Also, making each and every function check for NULL hurts performance, as it causes many (unnecessary) branches.

tautologicc avatar Nov 12 '19 20:11 tautologicc