[U-Boot-Users] [PATCH] v3 tftp: don't implicity trust the format of recevied packets

[Grant Likely]

From: Grant Likely <grant.likely at 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 at 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))