Hi Simon, On Sat, Sep 03, 2022 at 07:07:52PM +0200, Erik Auerswald wrote: > On Sat, Sep 03, 2022 at 05:39:45PM +0200, Simon Josefsson wrote: > > [...] > > did you notice some fuzzing report that wasn't fixed? > > I think the following reports have not yet been addressed: > > * Problems found in ftp (the code did not change since the reports): > > * Untrusted Pointer Dereference in domacro() at inetutils/ftp/domacro.c:186 > https://lists.gnu.org/archive/html/bug-inetutils/2021-12/msg00003.html > (https://savannah.gnu.org/bugs/?61722)
This one is a signed integer overflow leading to an out of bounds buffer read, possibly followed by an out of bounds buffer write. Simple reproducer: printf -- 'macdef x\n$999999999999999999\n\n$x\n' | ./ftp/ftp I have an initial fix, but want to take a better look at the issue and hopefully come up with a better solution. > * Infinite Loop in domacro at domacro.c:258 > https://lists.gnu.org/archive/html/bug-inetutils/2021-12/msg00005.html > https://savannah.gnu.org/bugs/?61724 > https://lists.gnu.org/archive/html/bug-inetutils/2021-12/msg00008.html This is a feature of the macro functionality in the ftp client. Macros can call macros which allows recursion. This trivially allows to create infinite recursion until the stack is exhausted and ftp crashes. Simple reproducer: printf -- 'macdef X\n$X\n\n$X\n' | ./ftp/ftp I am not sure if this is worth "fixing", and what exactly would constitute a "fix". > * A heap-buffer-overflow in another () at cmds.c:202 > https://lists.gnu.org/archive/html/bug-inetutils/2021-12/msg00016.html This one is a bit more involved. It seems as if the strcpy in line 266 can overflow the destination buffer, because that buffer can be changed during macro execution via the another() function. I have not yet understood the exact mechanism, and do not have a simple reproducer. Neither do I have a fix. > * NULL Pointer Dereference in setnmap() at cmds.c:2303 > https://lists.gnu.org/archive/html/bug-inetutils/2021-12/msg00004.html > https://savannah.gnu.org/bugs/?61723 > https://lists.gnu.org/archive/html/bug-inetutils/2021-12/msg00013.html The setnmap() function only works correctly with space characters as argument separators, but makeargv() works with both space and tab characters. This can result in a NULL pointer dereference when makeargv() splits arguments on a tab, but setnmap() searches for a space which is not there. Simple reproducer: printf -- 'nmap x\ty\n' | ./ftp/ftp I have an initial fix for the fuzzer input and the above reproducer, but also a slightly different reproducer that also crashes ftp. A fix similar to that for the fuzzer report fixes the second crash, but breaks some ftp client functionality, so this needs more work. That's all for now. Br, Erik -- The computing scientist’s main challenge is not to get confused by the complexities of his own making. -- Edsger W. Dijkstra