From: Grant Likely grant.likely@secretlab.ca
The TFTP OACK code trusts that the incoming packet is formated as ASCII text and can be processed by string functions. It also as a loop limit overflow bug where if the packet length is less than 8, it ends up looping over *all* of memory to find the 'blksize' string. This occurs because 'len' is an unsigned value, and 'len-8' is also calculated as unsigned which results in a huge loop limit.
This patch solves the problem by using memmem() to search for the sub string.
Signed-off-by: Grant Likely grant.likely@secretlab.ca --- Third version; this one makes sure simple_strtoul() doesn't run off the end of the buffer. Tested in my environment, but needs testing by others
net/tftp.c | 24 ++++++++++++++---------- 1 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/net/tftp.c b/net/tftp.c index fb2f505..460bfa0 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -238,9 +238,9 @@ TftpSend (void) static void TftpHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len) { + void * blksize; ushort proto; ushort *s; - int i;
if (dest != TftpOurPort) { #ifdef CONFIG_MCAST_TFTP @@ -272,22 +272,26 @@ TftpHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len)
case TFTP_OACK: #ifdef ET_DEBUG - printf("Got OACK: %s %s\n", pkt, pkt+strlen(pkt)+1); + printf("Got OACK:\n"); + print_buffer (0, pkt, 1, len, 16); #endif TftpState = STATE_OACK; TftpServerPort = src; - /* Check for 'blksize' option */ - for (i=0;i<len-8;i++) { - if (strcmp ((char*)pkt+i,"blksize") == 0) { - TftpBlkSize = (unsigned short) - simple_strtoul((char*)pkt+i+8,NULL,10); + + /* Search for the 'blksize' option */ + blksize = memmem(pkt, len, "blksize", 8); /* str + '\0' */ + if (blksize) { + blksize += 8; /* safe: "blksize\0" is inside pkt */ + + /* does blksize point at a terminated string? */ + if (memchr(pkt, '\0', (void*)pkt - blksize)) { + TftpBlkSize = simple_strtoul(blksize, NULL, 10); #ifdef ET_DEBUG - printf ("Blocksize ack: %s, %d\n", - (char*)pkt+i+8,TftpBlkSize); + printf("Blocksize ack: %d\n", TftpBlkSize); #endif - break; } } + #ifdef CONFIG_MCAST_TFTP parse_multicast_oack((char *)pkt,len-1); if ((Multicast) && (!MasterClient))