2 ms·

Not only that, but there appear to be potential buffer overflows which involve the data pulled from the wiki page[1]. I'm not sure why asprintf wasn't used here

Not only that, but there appear to be potential buffer overflows which involve the data pulled from the wiki page[1]. I'm not sure why asprintf wasn't used here (and in other places). The partial re-implementation of libc[2, 3] is also somewhat frightening. As are potential unchecked integer overflows when calculating a size to pass to malloc[4]. C is hard. -- [1] https://github.com/clibs/clib/blob/master/src/clib-install.c#L86 https://github.com/clibs/clib/blob/master/src/clib-install.c... [2] https://github.com/clibs/clib/blob/master/deps/fs/fs.c https://github.com/clibs/clib/blob/master/deps/fs/fs.c [3] https://github.com/clibs/clib/blob/master/deps/str-copy/str-copy.c https://github.com/clibs/clib/blob/master/deps/str-copy/str-... [4] https://github.com/clibs/clib/blob/master/deps/http-get/http-get.c#L19 https://github.com/clibs/clib/blob/master/deps/http-get/http...

gone35 12y ago
Oh dear... I'm just sight-linting so this might be wrong, but I'm certain the function executable in src/clib-install.c [1] can be easily overflown by providing a maliciously long package name or version --neither of which are validated at all [2] despite coming straight from the wiki page, as you point out. The function allocates 256-byte buffers to store a url and file name, respectively, but blindly fills them up using sprintf with non-validated user-supplied data, and without checking the resulting number of characters written, eg: char *url = malloc(256); if(NULL == file) goto e1; sprintf(file,"%s-%s.tar.gz",pkg->name,pkg->version); C is unforgiving indeed. Also what's up with all those (0 == count) yoda conditionals all over? Is that becoming a thing again? [1] https://github.com/clibs/clib/blob/master/src/clib-install.c https://github.com/clibs/clib/blob/master/src/clib-install.c [2] As far as I can tell, the package info from the wiki is dealt with by functions in deps/clib-package.c that, despite promising names like json_object_get_string_safe, do not ever validate the provided name or version string: https://github.com/clibs/clib/blob/master/deps/clib-package/clib-package.c https://github.com/clibs/clib/blob/master/deps/clib-package/...

gone35 12y ago

Oh dear... I'm just sight-linting so this might be wrong, but I'm certain the function executable in src/clib-install.c [1] can be easily overflown by providing a maliciously long package name or version --neither of which are validated at all [2] despite coming straight from the wiki page, as you point out. The function allocates 256-byte buffers to store a url and file name, respectively, but blindly fills them up using sprintf with non-validated user-supplied data, and without checking the resulting number of characters written, eg: char *url = malloc(256); if(NULL == file) goto e1; sprintf(file,"%s-%s.tar.gz",pkg->name,pkg->version); C is unforgiving indeed. Also what's up with all those (0 == count) yoda conditionals all over? Is that becoming a thing again? [1] https://github.com/clibs/clib/blob/master/src/clib-install.c https://github.com/clibs/clib/blob/master/src/clib-install.c [2] As far as I can tell, the package info from the wiki is dealt with by functions in deps/clib-package.c that, despite promising names like json_object_get_string_safe, do not ever validate the provided name or version string: https://github.com/clibs/clib/blob/master/deps/clib-package/clib-package.c https://github.com/clibs/clib/blob/master/deps/clib-package/...