autofs 5.1.9 snprintf() crash with -D_FORTIFY_SOURCE=3

Andreas Hasenack <[email protected]>
Newsgroups org.kernel.vger.autofs
Message-ID <CANYNYEEpJabu=qiQ2VUde6J4HUC9mQSgCGksbLQqGsr1-w5NYg@mail.gmail.com>
Hi,

in Ubuntu we are building most packages with -D_FORTIFY_SOURCE=3
nowadays, and we just got a bug report that 5.1.9 was crashing with a
buffer overflow warning. When rebuilt with -D_FORTIFY_SOURCE=2, it
does not crash.

Here is a small reproducer using a loop device. This is on kernel 6.8.0:

/etc/auto.master:
/- file,sun:/etc/auto.mp strictexpire

"strictexpire" is what triggers the crash.

/etc/auto.mp:
/mp defaults :/dev/loop0

# automount -f -d3
Starting automounter version 5.1.9, master map /etc/auto.master
using kernel protocol version 5.05
lookup_nss_read_master: reading master file /etc/auto.master
do_init: parse(sun): init gathered global options: (null)
lookup_read_master: lookup(file): read entry /-
master_do_mount: mounting /-
reading file map /etc/auto.mp
do_init: parse(sun): init gathered global options: (null)
*** buffer overflow detected ***: terminated
Aborted (core dumped)

gdb show this being in the snprintf call in lib/mounts.c when
",strictexpire" is being added to the autofs mount options string:
#9  0x00007ffff7dbaab4 in snprintf (__fmt=0x7ffff7dca232 "%s", __n=93,
__s=0x7fffec002c1c "") at
/usr/include/x86_64-linux-gnu/bits/stdio2.h:54
No locals.
#10 make_options_string (path=0x5555555b7d50 "/-", pipefd=6,
type=type@entry=0x7ffff7dca02b "direct", flags=2560) at
/usr/src/autofs-5.1.9-1ubuntu3/lib/mounts.c:764
        kver_major = <optimized out>
        kver_minor = 5
        options = 0x7fffec002bf0
"fd=6,pgrp=22935,minproto=5,maxproto=5,direct"
        max_len = 93
        len = 44
        new = <optimized out>
        __FUNCTION__ = "make_options_string"

lib/mounts.c:760
    /* maybe add ",strictexpire" */
    if (flags & MOUNT_FLAG_STRICTEXPIRE) {
        new = snprintf(options + len,
                   max_len, "%s", ",strictexpire");

I don't think this is actually overflowing options in this particular
case, but the max_len argument doesn't seem right, as that was the
original max size for options.

This patch stopped the crash, but there is a lot of C string
manipulation going on here and I would appreciate more eyes on this. I
 *think* the truncated and error checks are still ok after this:

--- a/lib/mounts.c
+++ b/lib/mounts.c
@@ -760,7 +760,7 @@ char *make_options_string(char *path, int pipefd,
    /* maybe add ",strictexpire" */
    if (flags & MOUNT_FLAG_STRICTEXPIRE) {
        new = snprintf(options + len,
-                  max_len, "%s", ",strictexpire");
+                  max_len - len, "%s", ",strictexpire");
        if (new < 0)
               goto error_out;
        len += new;
@@ -774,7 +774,7 @@ char *make_options_string(char *path, int pipefd,
    /* maybe add ",ignore" */
    if (flags & MOUNT_FLAG_IGNORE) {
        new = snprintf(options + len,
-                  max_len, "%s", ",ignore");
+                  max_len - len, "%s", ",ignore");
        if (new < 0)
               goto error_out;
        len += new;
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.