[PATCH] tftpd: strip leading slashes from the requested file name

Ali Ahmet Memis via busybox <[email protected]>
Newsgroups gmane.linux.busybox
Message-ID <[email protected]>
tftpd confines clients to the served directory by chroot'ing into the
DIR argument, added in 4e3beb2e1 to fix bug 4874. The requested name
itself is never checked for a leading slash, so the confinement depends
entirely on that chroot being in place.

DIR is optional. Without it there is no chroot and no chdir either, so
an absolute request is resolved from the real root. Serving files
relative to the current directory otherwise works fine in this mode,
which makes the escape easy to miss:

  $ cd /srv/tftp && tftpd          # inetd/udpsvd service
  $ tftp -g -r /etc/shadow HOST    # served

Uploads use the same name, and uploads are enabled unless -r is given.
A client can therefore overwrite any existing file the daemon can write
to, and with -c create new ones. tftpd usually runs as root, so this
reaches files such as a root-owned script or a crontab.

chroot also needs root, so a tftpd running as an unprivileged user
cannot use DIR at all: xchroot() fails and the daemon exits. That
setup has no way to confine clients.

Strip leading slashes instead of rejecting them. PXE clients routinely
ask for names like /pxelinux.0, and translating those into the served
directory is the behaviour bug 4874 asked for. With DIR the result is
unchanged, since xchroot() already chdir()s to the chroot root. Without
DIR both downloads and uploads are now resolved below the current
directory, and the existing dot check keeps rejecting ".." components.

   text    data     bss     dec     hex filename
   3998       0       0    3998     f9e networking/tftp.o before
   4019       0       0    4019     fb3 networking/tftp.o after
                           +21

Signed-off-by: Ali Ahmet Memis <[email protected]>
---
Both the read and the write were reproduced against a tftpd started
without DIR. Unrelated to this patch, udpsvd reads past the end of a
20-byte allocation while serving tftpd (caught by ASan). I am looking
into that separately

 networking/tftp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/networking/tftp.c b/networking/tftp.c
index b698a9288..fefb4b06b 100644
--- a/networking/tftp.c
+++ b/networking/tftp.c
@@ -944,6 +944,9 @@ int tftpd_main(int argc UNUSED_PARAM, char **argv)
 	G.block_buf_tail[0] = '\0';
 
 	local_file = G.block_buf + 2;
+	/* Strip leading slashes from client-supplied paths */
+	while (local_file[0] == '/')
+		local_file++;
 	if (local_file[0] == '.' || strstr(local_file, "/.")) {
 		error_msg = "dot in file name";
 		goto err;
-- 
2.55.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.