|
|
|
|
|
by pobrn
1063 days ago
|
|
It's a nice project, but - I guess as is tradition with the majority of C projects - it has resource leaks and buffer overflows. There is at least one resource leak, namely, `sfd` is not closed when certain `WARN()` invocations jump back to the `start` label; for example, when `gethostbyname()` fails (i.e. try a non-existent domain and observe that the sockets remain open with `lsof`). (It also seems to be leaked in the happy path, so presumably `SSL_set_fd()` does not take ownership.) And if the user simply presses enter, i.e. the input is an empty line, there is a buffer underflow in line 55 as `j` will be -1 initially. Also addr.sin_addr.s_addr = *((unsigned long*)he->h_addr_list[i]);
is a potential buffer overflow where `long` is 64 bits since only the first four bytes of `h_addr_list[i]` can be accessed, and also potentially misaligned for `unsigned long`; and it will also not work correctly on big endian platforms where `long` is 64 bits. Using `memcpy()` would have avoided all these problems. I am really confused as to how you arrived at the conclusion that "yes, this is the way to copy 4 bytes from A to B".This sounds rude, I know, and I apologize; I don't want to single you/this project out personally. I am just frustrated that even today there are people who work on C/C++ projects seemingly without having made GCC's -fanalyzer/asan/ubsan/valgrind/etc. an important part of their development workflow. |
|
Not to make any excuses, just for the context. I'm a beginner in C programming and I wrote only few small programs. My tooling is basically non existent. I will try to improve tho.