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 --- Wolfgang, please test this version. I think I've got it right now.
Cheers, g.
net/tftp.c | 19 +++++++++---------- 1 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/net/tftp.c b/net/tftp.c index fb2f505..e36d76f 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) { + char * blksize; ushort proto; ushort *s; - int i;
if (dest != TftpOurPort) { #ifdef CONFIG_MCAST_TFTP @@ -272,22 +272,21 @@ 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); + blksize = memmem(pkt, len, "blksize", 8); /* str + '\0' */ + if ((blksize) && (blksize + 8 < (char*)pkt + len)) { + TftpBlkSize = simple_strtoul(blksize + 8, 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))