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.
Regards
Santhosh
Attachment:
pam_krb5.c.format-overflow.patch
Description: Binary data