Subject: bin/10076: 16MB limit in tftp / tftpd
To: None <gnats-bugs@gnats.netbsd.org>
From: Reinoud Zandijk <reinoud@rangerover.netbsd.org>
List: netbsd-bugs
Date: 05/08/2000 14:33:19
>Number:         10076
>Category:       bin
>Synopsis:       Due to a malicious block counter type, the tftp/tftpd has a maximum 16Mb size limit
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon May 08 14:34:02 PDT 2000
>Closed-Date:
>Last-Modified:
>Originator:     Reinoud Zandijk
>Release:        Mon May  1 CEST 2000 <NetBSD-current source date>
>Organization:
	
>Environment:
	
System: NetBSD rangerover 1.4X NetBSD 1.4X (config.rangerover) #0: Mon May 1 13:33:05 CEST 2000 reinoud@rangerover:/usr/sources/anon-cvs/syssrc/sys/arch/sparc/compile/config.rangerover sparc


>Description:
	
Every tftp request made for a file bigger than 16Mb, the transfer stops when the 16Mb is reached ... this is due to the block counter.
This counter is a signed short in the protocol, and so the maximum number of blocks is 16Mb/512 bytes =  32768 and will then wrap to a
negative number. Since the tftpd deamon and the tftp client also keep a counter for block numbering and is declared as an `int',
this counter won't wrap and thus breaking the protocol, for it can't resynchronise anymore

The code is tested on NetBSD/i386 and NetBSD/sparc

>How-To-Repeat:
	
Fetch a file from a NetBSD system that is bigger than 16Mb... it won't give you more than the 16Mb and then just not respond anymore.

>Fix:
	
Patching the following files by changing the type of the `block' counter from `int' to `signed short' :

===================================================================
RCS file: /pub/NetBSD-CVS/basesrc/libexec/tftpd/tftpd.c,v
retrieving revision 1.18
diff -c -r1.18 tftpd.c
*** tftpd.c     1999/07/12 20:17:09     1.18
--- tftpd.c     2000/05/08 21:20:45
***************
*** 592,598 ****
        struct tftphdr *dp;
        struct tftphdr *ap;    /* ack packet */
        int size, n;
!       volatile int block;
  
        signal(SIGALRM, timer);
        dp = r_init();
--- 592,598 ----
        struct tftphdr *dp;
        struct tftphdr *ap;    /* ack packet */
        int size, n;
!       volatile signed short block;
  
        signal(SIGALRM, timer);
        dp = r_init();
***************
*** 662,668 ****
        struct tftphdr *dp;
        struct tftphdr *ap;    /* ack buffer */
        int n, size;
!       volatile int block;
  
        signal(SIGALRM, timer);
        dp = w_init();
--- 662,668 ----
        struct tftphdr *dp;
        struct tftphdr *ap;    /* ack buffer */
        int n, size;
!       volatile signed short block;
  
        signal(SIGALRM, timer);
        dp = w_init();
===================================================================

and 

===================================================================
RCS file: /pub/NetBSD-CVS/basesrc/usr.bin/tftp/tftp.c,v
retrieving revision 1.11
diff -c -r1.11 tftp.c
*** tftp.c      2000/01/21 17:08:36     1.11
--- tftp.c      2000/05/08 21:19:52
***************
*** 99,105 ****
        struct tftphdr *ap;        /* data and ack packets */
        struct tftphdr *dp;
        int n;
!       volatile int block, size, convert;
        volatile unsigned long amount;
        struct sockaddr_storage from;
        int fromlen;
--- 99,106 ----
        struct tftphdr *ap;        /* data and ack packets */
        struct tftphdr *dp;
        int n;
!       volatile signed short block;
!       volatile int size, convert;
        volatile unsigned long amount;
        struct sockaddr_storage from;
        int fromlen;
***************
*** 218,224 ****
        struct tftphdr *ap;
        struct tftphdr *dp;
        int n;
!       volatile int block, size, firsttrip;
        volatile unsigned long amount;
        struct sockaddr_storage from;
        int fromlen;
--- 219,226 ----
        struct tftphdr *ap;
        struct tftphdr *dp;
        int n;
!       volatile signed short block;
!       volatile int size, firsttrip;
        volatile unsigned long amount;
        struct sockaddr_storage from;
        int fromlen;
===================================================================

>Release-Note:
>Audit-Trail:
>Unformatted: