Hi,
there is a huge bug in the tftp daemon.
-- /usr/src/usr.sbin/tftpd/tftpd.c --
In tftp_open() on line 870, the daemon checks for options
(OACK) and handle them through the oack() function. Then,
it frees and NULLs the variable client->options.
oack() - l.1390 - does some stuff, and if an error happens,
client_free() is called with the client structure, which
frees this structure, including client->options, but does
not null it.
So, when returning after an error, client->options is freed
twice, causing a double-free and a crash.
I succeeded in making the server crash, by changing the
tsize and blksize options before transferring a small file
in localhost. In fact, if you look on line 1432, you'll see
that you just have to close the socket to make the server
crash, just after sending the OACK packet.
Typically, I just have to do:
$ tftp localhost
tftp> tsize 2
Tsize option on.
tftp> blksize 10
tftp> get test
[...wait ~ 10 sec...]
tftpd in free(): error: chunk is already free 0x1a658ca65f40
Here is a patch. I switched oack() to int, so that we can
handle errors and call nak() to alert the client before
closing the connection. nak() frees the client structure,
so all goes well.
Ok/Comments?
Index: tftpd.c
===================================================================
RCS file: /cvs/src/usr.sbin/tftpd/tftpd.c,v
retrieving revision 1.8
diff -u -r1.8 tftpd.c
--- tftpd.c 13 Jul 2012 02:31:46 -0000 1.8
+++ tftpd.c 15 Mar 2013 00:36:40 -0000
@@ -168,7 +168,7 @@
void tftp(struct tftp_client *, struct tftphdr *, size_t);
void tftp_open(struct tftp_client *, const char *);
void nak(struct tftp_client *, int);
-void oack(struct tftp_client *);
+int oack(struct tftp_client *);
void oack_done(int, short, void *);
void sendfile(struct tftp_client *);
@@ -876,8 +876,8 @@
goto error;
if (client->options) {
- oack(client);
-
+ if (oack(client) == -1)
+ goto error;
free(client->options);
client->options = NULL;
} else if (client->opcode == WRQ) {
@@ -1386,7 +1386,7 @@
/*
* Send an oack packet (option acknowledgement).
*/
-void
+int
oack(struct tftp_client *client)
{
struct opt_client *options = client->options;
@@ -1436,10 +1436,10 @@
oack_done, client);
event_add(&client->sev, &client->tv);
- return;
+ return 0;
error:
- client_free(client);
+ return -1;
}
int
