Security patch for weird pathnames

1 files changed, 212 insertions, 0 deletions

diff --git a/ftp/yale-tftpd/files/patch-ab b/ftp/yale-tftpd/files/patch-ab
new file mode 100644
index 0000000..cea9966
--- /dev/null
+++ b/ftp/yale-tftpd/files/patch-ab
@@ -0,0 +1,212 @@
+From: Christian.Schroeder@Inf-Technik.TU-Ilmenau.DE (Ch. Schroeder)
+Message-Id: <9510171319.AA19401@pegasus>
+Subject: yale tftpd
+To: pst@cisco.com
+Date: Tue, 17 Oct 1995 14:19:32 +0100 (MET)
+
+Hello Paul,
+
+Some days ago I foung the yale tftp daemon (3.0) on the INTERNET and I want to
+use it because I have to follow symbolic links from the tftp root directory
+to some bootfiles in other directories in the local filesystem.
+
+But I found, that there are some security holes (?) in the code, specially
+if the daemon checks the pathname of the requested file. That is dangereous
+if no default access rules are specified.
+
+i.e:
+
+config file:
+------------
+
+defaultDirectory                /tftpboot
+rootDirectory                   /tftpboot
+accessList              1       readonly        141.24.20.0     0.0.3.255
+defaultAccessList       1
+
+The following reqeusts were successfully and I think, that shouldn't be so.
+
+tftp> get /etc/passwd ./foo
+Error code 1: File not found
+tftp> get /tftpboot/../../etc/passwd ./foo
+Received 474 bytes in 0.3 seconds
+tftp> get ../../etc/passwd ./foo
+Received 474 bytes in 0.1 seconds
+tftp>
+
+I found also some Problems when I dont secify a rootDirectory.
+Specially the "../.." parts are only removed, if a root directory
+is specified AND the path starts with "/" AND the path doesn't start 
+with the virtual root directory (e.g. /tftpboot). restrict rules can
+be skipped with leading ../.. etc.
+Therefore I made some changes in tftpd.c an got better results.
+
+tftp> get /etc/passwd ./foo
+Error code 1: File not found
+tftp> get /tftpboot/../../etc/passwd ./foo
+Error code 2: Access violation
+tftp> get ../../etc/passwd ./foo
+Error code 1: File not found
+tftp>
+
+It would be very nice, if you could check my modifications and
+mail me your meaning back. (Excuse please my ugly english)
+
+Christian
+
+Here's the diff File ( diff -bw -c tftpd.c tftpd.c.patched ):
+
+*** tftpd.c	Tue Oct 17 14:09:01 1995
+--- tftpd.c.patched	Tue Oct 17 11:57:30 1995
+***************
+*** 459,475 ****
+  
+  	/* Rule 2:
+  	 */
+! 	if (tftpRootDirectory != 0 && IS_ROOTED(filename)) {
+  		char _tmp[1024];
+  		int maxPath;
+  		int rootLen;
+  
+! 		rootLen = strlen (tftpRootDirectory);
+  
+  		/* make sure the pathname doesn't already contain
+  		 * the virtual root.
+  		 */
+- 		if (strncmp(filename,tftpRootDirectory,rootLen) != 0) {
+  
+  			/* Insure our temporary space is big enough */
+  			maxPath = ((sizeof _tmp) - 1) - rootLen;
+--- 459,483 ----
+  
+  	/* Rule 2:
+  	 */
+! 	if ((tftpRootDirectory != 0 && IS_ROOTED(filename)) ||
+! 	    (tftpDefaultDirectory != 0 && IS_ROOTED(filename))) {
+  		char _tmp[1024];
++ 		char* realRootDir; 
+  		int maxPath;
+  		int rootLen;
+  
+! 		if (tftpRootDirectory != 0 ) {
+! 			realRootDir = tftpRootDirectory;
+! 		}
+! 		else {
+! 			realRootDir = tftpDefaultDirectory;
+! 		}
+  
++ 		rootLen = strlen (realRootDir);
++ 
+  		/* make sure the pathname doesn't already contain
+  		 * the virtual root.
+  		 */
+  
+  		/* Insure our temporary space is big enough */
+  		maxPath = ((sizeof _tmp) - 1) - rootLen;
+***************
+*** 481,486 ****
+--- 489,496 ----
+  			return EACCESS;
+  		}
+  
++ 		if (strncmp(filename,realRootDir,rootLen) != 0) {
++ 
+  			/* Squeeze out any '.' or '..' components */
+  			strcpy (tmpPath, filename);
+  			if (realPath (tmpPath, _tmp) < 0) {
+***************
+*** 492,511 ****
+  			/* Create the full pathname, prefixed by the
+  			 * virtual root.
+  			 */
+! 			strcpy (tmpPath, tftpRootDirectory);
+  			strcat (tmpPath, _tmp);
+  			filename = tmpPath;
+  		}
+  	}
+  
+  	/* Rule 3:
+  	 */
+! 	if (!IS_ROOTED(filename) && tftpDefaultDirectory == 0) {
+! 		strcpy (tmpPath, tftpRootDirectory);
+! 		strcat (tmpPath, "/");
+  		strcat (tmpPath, filename);
+  		filename = tmpPath;
+  	}
+  
+  	/* Check access lists */
+  	/* Rules 4&5:
+--- 502,554 ----
+  			/* Create the full pathname, prefixed by the
+  			 * virtual root.
+  			 */
+! 			strcpy (tmpPath, realRootDir);
+  			strcat (tmpPath, _tmp);
+  			filename = tmpPath;
+  		}
++ 		else {
++ 			/* Squeeze out any '.' or '..' components */
++ 		        strcpy (tmpPath, filename);
++                         if (realPath (tmpPath, _tmp) < 0) {
++                                 if (tftpDebugLevel > 1)
++                                         syslog (LOG_DEBUG, "realPath fails");
++                                 return EACCESS;
+                          }	
++ 			/* Create the full pathname */
++ 			strcpy (tmpPath,_tmp);
++ 			filename = tmpPath;
++ 			if (strncmp(filename,realRootDir,rootLen) != 0) {
++ 			    if (tftpDebugLevel > 1) {
++ 				syslog(LOG_DEBUG, "file=%s; invalid access denied", filename);
++ 				return EACCESS;
++ 	                    }	
++ 			}
++ 		}
++ 	}
+  
+  	/* Rule 3:
+  	 */
+! 	if ((!IS_ROOTED(filename)  && tftpRootDirectory != 0) ||
+! 	    (!IS_ROOTED(filename)  && tftpDefaultDirectory != 0)) {
+! 		char _tmp[1024];
+  		strcat (tmpPath, filename);
++ 	        /* Squeeze out any '.' or '..' components */
++                         strcpy (tmpPath, filename);
++                         if (realPath (tmpPath, _tmp) < 0) {
++                                 if (tftpDebugLevel > 1)
++                                         syslog (LOG_DEBUG, "realPath fails");
++                                 return EACCESS;
++                         }
++ 		if ( tftpDefaultDirectory == 0 ) {
++ 			strcpy (tmpPath, tftpRootDirectory);
++ 		}
++ 		else {
++ 			strcpy (tmpPath, tftpDefaultDirectory);
++ 		}
++ 		strcat (tmpPath, _tmp);
+  		filename = tmpPath;
+  	}
++ 
+  
+  	/* Check access lists */
+  	/* Rules 4&5:
+-- 
+
+                                             ***
+                                            (o o)
+ ---------------------------------------ooO--(_)--Ooo----------------------
+|                           |                                              |
+|                           | Christian Schroeder (Dr.-Ing.)               |
+|                           |                                              |
+|        _/_/_/     _/_/_/  | Technische Universitaet Ilmenau              |
+|      _/    _/  _/     _/  | Fakultaet Elektrotechnik/Informationstechnik |
+|    _/          _/         | Mikroelektronische Schaltungen u. Systeme    |
+|   _/           _/_/_/     | Postfach 0565                                |
+|  _/                 _/    | 98684 ILMENAU                                |
+|  _/     _/  _/      _/    |                                              |
+|   _/_/_/     _/_/_/       | Phone  : +49 (0) 3677/69-1165/1168/1169      |
+|                           | FAX    : +49 (0) 3677/69-1163                |
+|                           | E-Mail : schroeder@Inf-Technik.TU-Ilmenau.DE |
+ --------------------------------------------------------------------------
+
+