Re: autofs 5.1.9 snprintf() crash with -D_FORTIFY_SOURCE=3

On 18/4/24 11:46, Ian Kent wrote:
> On 18/4/24 01:28, Andreas Hasenack wrote:
>> 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.
>
> Ha, I had report of this just the other day and I missed that obvious 
> stupid mistake so thanks
>
> for reporting it.
>
>
> I chose to change the snprintf() to strcat() because it clearly wasn't 
> going to overflow as
>
> the calculated maximum size was sure to be larger than the contents.
>
>
> fyi, what I used was this but I'll update the description to include 
> the max_len usage mistake
>
> you have pointed out (and push the change to the repo. in case you 
> prefer to use it in Ubuntu):

I have pushed this change and a couple of others to the repo.

Unfortunately I made a grammatical mistake in the description which will 
have to remain since I don't want

to force push a change to fix it because that might cause problems for 
people with existing repo. copies.


Ian