Today I spent a while fixing some more segfault bugs. I guess that this work qualifies as either fixing RC bugs, or potential security bugs.
Anyway I did an NMU of libpam-tmpdir a while back to fix all but one of the open bugs against it.
I provided a patch for #461625 yelp: segfault while loading info documentation, which fixes the symptoms of bad info-parsing, and avoids the segfault.
I also looked into the #466771 busybox cpio: double free or corruption during cpio extraction of hardlinks - but it turns out that was already fixed in Sid.
Finally I found a segfault bug open against ftp:
- ftp: GET segfaults. Buffer overflow?
- Buffer overflow: comman buffer size is not checked (Almost certainly a duplication bug.)
To reproduce this bug run:
skx@gold:~$ ftp ftp.debian.org 220 saens.debian.org FTP server (vsftpd) Name (ftp.debian.org:skx): anonymous 331 Please specify the password Password: [email protected] ftp> cd debian/doc 250 Directory successfully changed. ftp> get dedication-2.2.cn.txt dedication-2.2.de.txt dedication-2.2.es.txt .. local: dedication-2.2.de.txt remote: dedication-2.2.cn.txt Segmentation fault
You need to repeat the arguments about 50 times. But keep adding more and more copies of the three files to the line until you get the crash.
It isn't interesting as a security issue as it is client side only; but as a trivially reproducable issue it becomes fun to solve.
Lets build it with debugging information, and run it again. Here is what we see:
Core was generated by `./ftp/ftp ftp.debian.org'.
Program terminated with signal 11, Segmentation fault.
#0 0x00002b85ad77f1cf in fwrite () from /lib/libc.so.6
(gdb) up
#1 0x0000000000408c3e in command (fmt=0x40dd15 "TYPE %s") at ftp.c:366
366 fputs("\r\n", cout);
(gdb) up
#2 0x0000000000402c3e in changetype (newtype=3, show=)
at cmds.c:348
348 comret = command("TYPE %s", p->t_mode);
(gdb) up
#3 0x000000000040a569 in recvrequest (cmd=,
local=0x623d10 "dedication-2.2.de.txt",
remote=0x6238d4 "dedication-2.2.cn.txt", lmode=0x40e310 "w",
printnames=) at ftp.c:935
935 changetype(type, 0);
OK so things look trashed, and not in the middle of a copy/sprintf/similar. i.e. there is no obvious problem.
Lets take a step back from things. We know that the crash occurs when we send a long command line. Looking over the code we see the fucntion main.c:makeargv(). This attempts to split the input command line string into an array of tokens.
Interestingly we see this:
char **
makeargv(int *pargc, char **parg)
{
static char *rargv[20];
int rargc = 0;
char **argp;
I wonder what happens if we set the 20 to 2048? Good guess. The crash no longer occurs. (Though I'm sure it would if you entered enough tokens...)
So we know that the crash is relating to the number of space-separated tokens upon the command line. If we increase the limit we'll be fine. But of course we want to fix it properly. There are two ways forward:
- Abort handling a line if there are >15 "space" characters on the line.
- Recode the makeargv function to work properly.
I did eventually submit a patch to the bug report which uses dynamic memory allocation, and should always work. Job done.
I mailed the maintainer of FTP and said unless I heard differently I'd NMU and cleanup the package in a week.
All being well this entry will be nicely truncated in the RSS feeds as support for the <cut> tag was the main new feature in my previous upload of chronicle - the blog compiler I use/wrote/maintain.
ObQuote: Razor Blade Smile
Tags: chronicle, debian, ftp, public bugfixing No comments