Fixing random crashes during "dict" commands due to buffer size miscalculation

[benryves]

I believe there are two places where buffer sizes are miscalculated in main.c, where they should be "strlen(str) + 1" but a misplaced parenthesis makes this "strlen(str + 1)" which results in a buffer two bytes too small. On Windows at least this means the program will randomly crash when performing "dict" commands, and gdb consistently reports "Heap block at x modified at y past requested size of n" when using the "dict" commands.

(I also had to make some minor other tweaks to get the program building without errors and warnings, but these should not affect other platforms).

[brijohn]

Ben,

Thanks for the patch, it does indeed apear i had the + 1 inside the call to strlen which is definitely wrong. Its a wonder i necer ended up with segfaults on my linux system in the past.

Before I merge this though there are a few issues:

• The arpa/inet.h files aren't unused, that is where the ntoh* and hton* functions are defined on linux/unix systems.
• and at least on linux offsetof is defined in stddef.h so you need to include that in the list.h when you use offsetof.

[benryves]

Thank you very much for the feedback (and such an invaluable tool in the first place!)

According to the MS documentation the header to include for the ntoh* and hton* functions is Winsock2.h so I've restored the arpa/inet.h include with an #if defined(MINGW32) to select which include file to use. I hope that's a suitable solution?

I have also added #include <stddef.h> to list.h.

[benryves]

I think there's a buffer overflow with the way usernames are automatically chosen in the "dict auth" command. This command doesn't work for me (I always needed to use "dict auth benryves") and I think the buffer overflow is the reason.

This retrieves user.inf from the dictionary and then uses sscanf to pull the username from the buffer. However in my case buffer is exactly 8 characters long and contains "benryves" (with no terminator or whitespace after it) which means sscanf will continue reading junk past the end of the username and copying it to the allocated "user" string (which is only one byte longer than the original buffer length, so will likely overflow).

I could think of a couple of fixes to continue using sscanf - either xrealloc() buffer to be one byte longer and append a terminator so sscanf can always reach the end of the username (xrealloc may fail and extra error handling logic would be required) or to put a width specifier in sscanf's %s format string based on the length of the received buffer.

In the end I just used a for loop that would copy buffer to user character by character until either len is reached or the character is <= ' ' (to detect whitespace) as it seemed like the simplest option.