svn commit: r314780 - head/lib/libpam/modules/pam_exec
Lawrence Stewart
lstewart at freebsd.org
Sun Mar 12 17:42:00 UTC 2017
On 13/03/2017 04:30, Pedro Giffuni wrote:
>
>
> On 3/12/2017 12:14 PM, Lawrence Stewart wrote:
>> Hi Pedro,
>>
>> On 07/03/2017 02:45, Pedro F. Giffuni wrote:
>>> Author: pfg
>>> Date: Mon Mar 6 15:45:46 2017
>>> New Revision: 314780
>>> URL: https://svnweb.freebsd.org/changeset/base/314780
>>>
>>> Log:
>>> libpam: extra bounds checking through reallocarray(3).
>>> Reviewed by: des
>>> MFC after: 1 week
>>>
>>> Modified:
>>> head/lib/libpam/modules/pam_exec/pam_exec.c
>>>
>>> Modified: head/lib/libpam/modules/pam_exec/pam_exec.c
>>> ==============================================================================
>>>
>>> --- head/lib/libpam/modules/pam_exec/pam_exec.c Mon Mar 6
>>> 15:42:03 2017 (r314779)
>>> +++ head/lib/libpam/modules/pam_exec/pam_exec.c Mon Mar 6
>>> 15:45:46 2017 (r314780)
>>> @@ -138,7 +138,7 @@ _pam_exec(pam_handle_t *pamh __unused,
>>> nitems = sizeof(env_items) / sizeof(*env_items);
>>> /* Count PAM return values put in the environment. */
>>> nitems_rv = options->return_prog_exit_status ? PAM_RV_COUNT : 0;
>>> - tmp = realloc(envlist, (envlen + nitems + 1 + nitems_rv + 1) *
>>> + tmp = reallocarray(envlist, envlen + nitems + 1 + nitems_rv + 1,
>>> sizeof(*envlist));
>>> if (tmp == NULL) {
>>> openpam_free_envlist(envlist);
>>>
>> This commit breaks pam_exec for me... without this change I see the
>> expected PAM_* environment variables from my execed script, but with
>> this change I no longer see any of them.
> Thanks for the report.
>
> It seems strange this can cause any failure. Perhaps there is a latent
> overflow here and we have been living with it? I will revert while it is
> investigated.
>
> BTW, the "nitems" variable may conflict with nitems() in sys/param.h.
I don't think so. I manually ran the compile step in
/usr/src/lib/libpam/modules/pam_exec replacing -o<out> with -E per:
cc -DOPENPAM_STATIC_MODULES -O2 -pipe -I/usr/src/contrib/openpam/include
-I/usr/src/lib/libpam -DOPENPAM_DEBUG -MD -MF.depend.pam_exec.o
-MTpam_exec.o -std=gnu99 -fstack-protector-strong -Wsystem-headers
-Werror -Wno-pointer-sign -Wno-empty-body -Wno-string-plus-int
-Wno-unused-const-variable -Wno-tautological-compare -Wno-unused-value
-Wno-parentheses-equality -Wno-unused-function -Wno-enum-conversion
-Wno-unused-local-typedef -Wno-address-of-packed-member -Wno-switch
-Wno-switch-enum -Wno-knr-promoted-parameter -Wno-parentheses
-Qunused-arguments -c pam_exec.c -E | vim -
and the preprocessed code in question looks sane (included a few lines
of context either side):
envlist = pam_getenvlist(pamh);
for (envlen = 0; envlist[envlen] != ((void *)0); ++envlen)
;
nitems = sizeof(env_items) / sizeof(*env_items);
nitems_rv = options->return_prog_exit_status ? 24 : 0;
tmp = reallocarray(envlist, envlen + nitems + 1 + nitems_rv + 1,
sizeof(*envlist));
if (tmp == ((void *)0)) {
openpam_free_envlist(envlist);
return (PAM_BUF_ERR);
}
Cheers,
Lawrence
More information about the svn-src-head
mailing list