Re: pam_krb5.c - -Werror=format-overflow

[christos%astron.com@localhost (Christos Zoulas)]

In article <CACQjD2FQ=7ABLqxrF7GrO8GvRm03OE55G_mRe4zAM4080h_Kiw%mail.gmail.com@localhost>,
Santhosh Raju  <fox%netbsd.org@localhost> wrote:
>-=-=-=-=-=-
>
>Hello
>
>While working with kamil@ on MKLIBCSANITIZER based build, I came
>across a compile error in pam_krb5.c
>
>--- dependall-pam_krb5 ---
>/home/fox/projects/netbsd/obj-wip/destdir.amd64/usr/include/security/pam_modules.h:
>In function 'pam_sm_setcred':
>/home/fox/projects/netbsd/src-wip/lib/libpam/modules/pam_krb5/pam_krb5.c:489:6:
>error: null destination pointer [-Werror=format-overflow=]
>      sprintf(p, "%d", getpid());
>      ^~~~~~~~~~~~~~~~~~~~~~~~~~
>/home/fox/projects/netbsd/src-wip/lib/libpam/modules/pam_krb5/pam_krb5.c:485:6:
>error: null destination pointer [-Werror=format-overflow=]
>      sprintf(p, "%d", pwd->pw_uid);
>      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>This error is reproducible in gcc version 8.3.0, but not in gcc
>version 5.5.0 in NetBSD 8.1/amd4 (my build / test environment). It
>was also reproducible under gcc version 7.4.0 in Ubuntu (thanks to
>help from Ayushi who helped me out in testing the scenario there).
>
>After some contemplation on how to resolve this issue, I decided to
>replace sprintf(3) with snprinft(3) since this would just truncate the
>output in case of an overflow. I have attached a patch showing the
>changes.
>
>The "n" for the snprintf(3) has been chosen as PATH_MAX + 16 since
>this is the argument passed into to the calloc(3) when allocating
>space for "p". In addition to this (not shown in the patch), I think
>we should also keep track of the remaining buffer size left after each
>snprintf(3) invocation and break out of the loop when buffer size
>becomes less than 1 to prevent a buffer overflow.
>
>for example
>
>buffer_size = PATH_MAX  + 16
>
>...
>
>buffer_size -= strlen(p)
>p += strlen(p)
>
>Since this change is in one of the userland tools which is an
>integral part in the NetBSD src tree, I seek advice on this change and it's
>potential implications and if this is the right way to solve it. The
>other option would be to add a -Wno-error=format-overflow and leave
>the code as is.
>
>On a side note, this error happens when the optimization flag is set
>to -O2 and the compile goes through when -O0 is passed in as the flag.

Thanks, should be fixed now.

christos