Fix 256 and more #257
Fix 256 and more #257
Conversation
src/main.c
Outdated
| snprintf(path=pbuf, sizeof(pbuf), "%s/%s", prefix, dll_name); | ||
| if (path[0] != '/'){ | ||
| path = realpath(path, NULL); | ||
| strncpy(pbuf, path, sizeof(pbuf)); |
There was a problem hiding this comment.
strncpy() doesn't do what you think it does. use snprintf(pbuf, sizeof pbuf, "%s", path) instead
There was a problem hiding this comment.
I'm willing to concede you're correct, but I'm not seeing why strncpy is wrong. Could you give me a clue? Upon review I realize that it should be size(pbuf)-1 though. I'll change it anyway, since upon further thought the reason I was using it was dumb.
There was a problem hiding this comment.
strncpy() has 2 properties ppl aren't usually aware of
- strncpy doesn't zero-terminate the resulting string if the strlen is >= buffer length
- strncpy always zero-pads the entire buffer, means if the buffersize is 64K and the string is 5 bytes, it will write 64K characters
while 2) is just very ineffective, 1) can turn into an exploitable security issue
src/debug.h
Outdated
| @@ -3,8 +3,8 @@ | |||
|
|
|||
| #ifdef DEBUG | |||
| # include <stdio.h> | |||
| # define PSTDERR(fmt, args...) do { dprintf(2,fmt, ## args); } while(0) | |||
| # define PDEBUG(fmt, args...) PSTDERR("DEBUG:"fmt, ## args) | |||
| # define PSTDERR(fmt, args...) do { dprintf(2,fmt, getpid(), ## args); } while(0) | |||
There was a problem hiding this comment.
there's one occurence of PSTDERR() in debug.c. i suspect that one will get the arguments shuffled around unexpectedly with this change. i guess it'd be safer to put the getpid call directly into the PDEBUG macro, rather than spreading the functionality over 2 macros depending on each other
There was a problem hiding this comment.
Hah! Good catch, that was unintentional... 3am coding...
|
welcome back! i made some comment, apart from that it looks good |
… chdir is followed by a process fork, which can cause the library to not be found.
|
Hey and thanks! I changed the strncpy to snprintf, but I'm not seeing why it was problematic (there was an issue where the len param should've been |
|
@crass did you see my response re: strncpy() ? |
This fixes issue #256 and another issue where calling the proxychains4 binary with a relative path can cause some child processes to fail to load the library under certain conditions. This can happen when not using the wrapper too if the value of the envvar is not an absolute path. I don't fix that issue because I didn't want to write the code to potentially need to search through a list of paths to find our library and make it absolute. The caller should know to make the path absolute.
There is also an update to PDEBUG to always output the PID in a prefix. This is helpful for multi-process applications.