Bug#905647: tftpd-hpa: Logic flaw in macro expansion in remap for \x causes malfunction and memory corruption

[Tomas Forsman]

Package: tftpd-hpa
Version: 5.2+20150808-1+b1
Severity: normal
Tags: upstream patch

Dear Maintainer,

   * What led up to the situation?

I wanted to use the remap function with macro \x (hex ip)

   * What exactly did you do (or not do) that was effective (or
     ineffective)?

Start with --map-file /somefile
In '/somefile', I have a rule:
r       /fai/efi/grub.cfg       /fai/efi/grub.cfg.d/\x

   * What was the outcome of this action?

Complaints from glibc about memory corruption, segfaults. Valgrind gave
54 errors. No working file.

   * What outcome did you expect instead?

Macro expansion and working file.


In tftpd/tftpd.c the function rewrite_macros is first called with
argument 'output' = NULL, asking for "how long will the string be".
A logic flaw makes the function return l=0 for macro 'x'. 0 bytes (+1
for string-ending NULL) is allocated, and then the actual macro
expansion is performed and stored there (8 bytes for ipv4, 32 for ipv6),
overwriting other memory.

Patch moves the 'if (output)' to only include the string copying, not
length calculation.

Same flaw exists in upstreams version 5.2 and Ubuntu version
5.2+20150808-1ubuntu3.

-- System Information:
Debian Release: 9.5
  APT prefers stable-updates
  APT policy: (500, 'stable-updates'), (500, 'stable')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 4.9.0-7-amd64 (SMP w/3 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), 
LANGUAGE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/bash
Init: systemd (via /run/systemd/system)

Versions of packages tftpd-hpa depends on:
ii  adduser                3.115
ii  debconf [debconf-2.0]  1.5.61
ii  libc6                  2.24-11+deb9u3
ii  libwrap0               7.6.q-26

tftpd-hpa recommends no packages.

Versions of packages tftpd-hpa suggests:
pn  pxelinux  <none>

-- debconf information excluded

--- tftpd.c.orig        2015-08-08 06:24:45.000000000 +0200
+++ tftpd.c     2018-08-07 15:03:53.242504752 +0200
@@ -1365,24 +1365,26 @@
             return strlen(p);
 
     case 'x':
-        if (output) {
-            if (from.sa.sa_family == AF_INET) {
+        if (from.sa.sa_family == AF_INET) {
+            l = 8;
+            if (output) {
                 sprintf(output, "%08lX",
                     (unsigned long)ntohl(from.si.sin_addr.s_addr));
-                l = 8;
+            }
 #ifdef HAVE_IPV6
-            } else {
-                unsigned char *c = (unsigned char *)SOCKADDR_P(&from);
-                p = tb;
-                for (l = 0; l < 16; l++) {
-                    sprintf(p, "%02X", *c);
-                    c++;
-                    p += 2;
-                }
+        } else {
+            unsigned char *c = (unsigned char *)SOCKADDR_P(&from);
+            p = tb;
+            for (l = 0; l < 16; l++) {
+                sprintf(p, "%02X", *c);
+                c++;
+                p += 2;
+            }
+            l = strlen(tb);
+            if (output) {
                 strcpy(output, tb);
-                l = strlen(tb);
-#endif
             }
+#endif
         }
         return l;