antirez / sds

Simple Dynamic Strings library for C
BSD 2-Clause "Simplified" License
4.89k stars 473 forks source link

Null Dereferences v2.0.0 #99

Open PatriciaSVMonteiro opened 6 years ago

PatriciaSVMonteiro commented 6 years ago

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).

hu55a1n1 commented 5 years ago

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.

tautologicc commented 4 years ago

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.