Bug 189955 - makecontext(3) argument validation seems broken on i386 and powerpc
Summary: makecontext(3) argument validation seems broken on i386 and powerpc
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: i386 (show other bugs)
Version: CURRENT
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-i386 mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-05-19 17:10 UTC by Peter Holm
Modified: 2017-12-31 22:23 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Holm freebsd_committer 2014-05-19 17:10:01 UTC
The argc validation seems broken:

$ grep -rw NCARGS /usr/include
/usr/include/sys/param.h:#define        NCARGS          ARG_MAX         /* max bytes for an exec function */
$ egrep "argc < 0.*argc" src/lib/libc/*/gen/makecontext.c              
src/lib/libc/amd64/gen/makecontext.c:   else if ((argc < 0) || (argc > 6) || (ucp->uc_stack.ss_sp == NULL) ||
src/lib/libc/i386/gen/makecontext.c:    else if ((argc < 0) || (argc > NCARGS)) {
src/lib/libc/ia64/gen/makecontext.c:    if (argc < 0 || argc > 8 || ucp == NULL ||
src/lib/libc/mips/gen/makecontext.c:    if (argc < 0 || argc > 6 || ucp == NULL ||
src/lib/libc/powerpc/gen/makecontext.c: if ((ucp == NULL) || (argc < 0) || (argc > NCARGS)
src/lib/libc/powerpc64/gen/makecontext.c:       if ((ucp == NULL) || (argc < 0) || (argc > NCARGS)
src/lib/libc/sparc64/gen/makecontext.c: if ((argc < 0) || (argc > 6) ||
$
Comment 1 Bruce Evans freebsd_committer 2014-05-20 12:38:50 UTC
On Mon, 19 May 2014, Peter Holm wrote:

>> Description:
> The argc validation seems broken:

Why should it do any validation at all?  Its man page documents (much more
clearly than most man pages) that the behaviour for invalid args is
undefined.  It can't possibly do any useful validation, since ucp points
to mostly-opaque data.  All it can do is check that ucp is not a null pointer
and that a bit of data pointed to it is reasonable, and that argc is
reasonable.  It also cannot check that the extra args given by argc
are reasonable.

> $ grep -rw NCARGS /usr/include
> /usr/include/sys/param.h:#define        NCARGS          ARG_MAX         /* max bytes for an exec function */

Bogus.  makecontext() isn't an exec function.  A random integer would work
almost as well here.  An arbitrary limit gratuitously restricts
makecontext().  Negative and small positive values for the limit would
just restrict it enough to break it.  It mallocs storage related to
argc.  A large argc can cause the size calculations for the malloc() to
fail, but that is just a special case of undefined behaviour from
invalid args.  The behaviour is undefined unless argc matches the actual
number of args.  argc is just a helper for the variadic args.  The limit
on the number of args for makecontext() is equally poorly documented to
the limit on the number of args for a C function.  The C standard requires
some minimum limit that is too small, and IIRC it requires the actual
limit to be documented, but documentation about this is hard to find.
In practice, the limit is unreachable except on 16-bit systems (and
when someone uses ulimit(1) to demonstrate brokenness).

> $ egrep "argc < 0.*argc" src/lib/libc/*/gen/makecontext.c
> src/lib/libc/amd64/gen/makecontext.c:   else if ((argc < 0) || (argc > 6) || (ucp->uc_stack.ss_sp == NULL) ||

6 is an even more gratuitously restrictive limit.

> src/lib/libc/i386/gen/makecontext.c:    else if ((argc < 0) || (argc > NCARGS)) {
> src/lib/libc/ia64/gen/makecontext.c:    if (argc < 0 || argc > 8 || ucp == NULL ||
> src/lib/libc/mips/gen/makecontext.c:    if (argc < 0 || argc > 6 || ucp == NULL ||
> src/lib/libc/powerpc/gen/makecontext.c: if ((ucp == NULL) || (argc < 0) || (argc > NCARGS)
> src/lib/libc/powerpc64/gen/makecontext.c:       if ((ucp == NULL) || (argc < 0) || (argc > NCARGS)
> src/lib/libc/sparc64/gen/makecontext.c: if ((argc < 0) || (argc > 6) ||

The only bugs I see here are:
- the existence of these checks
- the style bugs in them:
   - excessive parentheses for amd64, i386, powerpc and sparc64
   - other gratuitous differences, like having the ucp == NULL check first
     for powerpc and elsewhere for amd64, i386 and sparc64.  The complete
     file differences are even larger due to different birthplaces and not
     much convergence.

Bruce
Comment 2 Eitan Adler freebsd_committer freebsd_triage 2017-12-31 07:59:18 UTC
For bugs matching the following criteria:

Status: In Progress Changed: (is less than) 2014-06-01

Reset to default assignee and clear in-progress tags.

Mail being skipped