[Bug network/20790] New: rpcgen buffer overrun in get_prog_declaration

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bug network/20790] New: rpcgen buffer overrun in get_prog_declaration

glaubitz at physik dot fu-berlin.de
https://sourceware.org/bugzilla/show_bug.cgi?id=20790

            Bug ID: 20790
           Summary: rpcgen buffer overrun in get_prog_declaration
           Product: glibc
           Version: 2.24
            Status: NEW
          Severity: normal
          Priority: P2
         Component: network
          Assignee: unassigned at sourceware dot org
          Reporter: jsm28 at gcc dot gnu.org
  Target Milestone: ---

Building with GCC 7 produces an error building rpcgen:

rpc_parse.c: In function 'get_prog_declaration':
rpc_parse.c:543:25: error: may write a terminating nul past the end of the
destination [-Werror=format-length=]
     sprintf (name, "%s%d", ARGNAME, num); /* default name of argument */
                     ~~~~^
rpc_parse.c:543:5: note: format output between 5 and 14 bytes into a
destination of size 10
     sprintf (name, "%s%d", ARGNAME, num); /* default name of argument */
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

That buffer overrun is for the case where the .x file declares a program with a
million arguments.  The strcpy two lines above can generate a buffer overrun
much more simply for a long argument name; the following test does it:

program TPROG { version TVERS { int FUNC(int
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)
= 1; } = 1; } = 1;

Testing a patch.

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug network/20790] rpcgen buffer overrun in get_prog_declaration

glaubitz at physik dot fu-berlin.de
https://sourceware.org/bugzilla/show_bug.cgi?id=20790

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fweimer at redhat dot com
              Flags|                            |security-

--- Comment #1 from Florian Weimer <fweimer at redhat dot com> ---
This does not cross a trust boundary, so flagging as security-.

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug network/20790] rpcgen buffer overrun in get_prog_declaration

glaubitz at physik dot fu-berlin.de
In reply to this post by glaubitz at physik dot fu-berlin.de
https://sourceware.org/bugzilla/show_bug.cgi?id=20790

--- Comment #2 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, master has been updated
       via  5874510faaf3cbd0bb112aaacab9f225002beed1 (commit)
      from  116ae4fe5a70d99cf06088f0222a5913996f7f70 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=5874510faaf3cbd0bb112aaacab9f225002beed1

commit 5874510faaf3cbd0bb112aaacab9f225002beed1
Author: Joseph Myers <[hidden email]>
Date:   Tue Nov 8 23:44:51 2016 +0000

    Fix rpcgen buffer overrun (bug 20790).

    Building with GCC 7 produces an error building rpcgen:

    rpc_parse.c: In function 'get_prog_declaration':
    rpc_parse.c:543:25: error: may write a terminating nul past the end of the
destination [-Werror=format-length=]
         sprintf (name, "%s%d", ARGNAME, num); /* default name of argument */
                         ~~~~^
    rpc_parse.c:543:5: note: format output between 5 and 14 bytes into a
destination of size 10
         sprintf (name, "%s%d", ARGNAME, num); /* default name of argument */
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

    That buffer overrun is for the case where the .x file declares a
    program with a million arguments.  The strcpy two lines above can
    generate a buffer overrun much more simply for a long argument name.

    The limit on length of line read by rpcgen (MAXLINESIZE == 1024)
    provides a bound on the buffer size needed, so this patch just changes
    the buffer size to MAXLINESIZE to avoid both possible buffer
    overruns.  A testcase is added that rpcgen does not crash with a
    500-character argument name, where it previously crashed.

    It would not at all surprise me if there are many other ways of
    crashing rpcgen with either valid or invalid input; fuzz testing would
    likely find various such bugs, though I don't think they are that
    important to fix (rpcgen is not that likely to be used with untrusted
    .x files as input).  (As well as fuzz-findable bugs there are probably
    also issues when various int variables get overflowed on very large
    input.)  The test infrastructure for rpcgen-not-crashing tests would
    need extending if tests are to be added for cases where rpcgen should
    produce an error, as opposed to cases where it should succeed.

    Tested for x86_64 and x86.

        [BZ #20790]
        * sunrpc/rpc_parse.c (get_prog_declaration): Increase buffer size
        to MAXLINESIZE.
        * sunrpc/bug20790.x: New file.
        * sunrpc/Makefile [$(run-built-tests) = yes] (rpcgen-tests): New
        variable.
        [$(run-built-tests) = yes] (tests-special): Add $(rpcgen-tests).
        [$(run-built-tests) = yes] ($(rpcgen-tests)): New rule.

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog          |    9 +++++++++
 sunrpc/Makefile    |   11 +++++++++++
 sunrpc/bug20790.x  |    1 +
 sunrpc/rpc_parse.c |    2 +-
 4 files changed, 22 insertions(+), 1 deletions(-)
 create mode 100644 sunrpc/bug20790.x

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug network/20790] rpcgen buffer overrun in get_prog_declaration

glaubitz at physik dot fu-berlin.de
In reply to this post by glaubitz at physik dot fu-berlin.de
https://sourceware.org/bugzilla/show_bug.cgi?id=20790

Joseph Myers <jsm28 at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED
   Target Milestone|---                         |2.25

--- Comment #3 from Joseph Myers <jsm28 at gcc dot gnu.org> ---
Fixed for 2.25.

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug network/20790] rpcgen buffer overrun in get_prog_declaration

glaubitz at physik dot fu-berlin.de
In reply to this post by glaubitz at physik dot fu-berlin.de
https://sourceware.org/bugzilla/show_bug.cgi?id=20790

--- Comment #5 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, hjl/pr21120/2.24 has been created
        at  7cfdcd57fb1ab3396b1e871b54f46b13acdf4b9e (commit)

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=7cfdcd57fb1ab3396b1e871b54f46b13acdf4b9e

commit 7cfdcd57fb1ab3396b1e871b54f46b13acdf4b9e
Author: Stefan Liebler <[hidden email]>
Date:   Fri Feb 24 13:30:13 2017 +0100

    Get rid of duplicate const declaration specifier warning in
tst-resolv-qtypes.c.

    Compiling resolv/tst-resolv-qtypes.c with GCC 7 results in:
    tst-resolv-qtypes.c:53:14: error: duplicate ‘const’ declaration specifier
[-Werror=duplicate-decl-specifier]
     static const const char *domain = "www.example.com";

    This patch removes the duplicate const and makes domain a const pointer
    to const char literal.

    ChangeLog:

        * resolv/tst-resolv-qtypes.c (domain):
        Change type to const pointer to const char.

    (cherry picked from commit d4f94368a96541db2b38b6535402a941f5aff975)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=4308d3fc8fab5b05707b7cc2f11cdb03cd89a16a

commit 4308d3fc8fab5b05707b7cc2f11cdb03cd89a16a
Author: Joseph Myers <[hidden email]>
Date:   Wed Jan 4 23:33:31 2017 +0000

    Fix string/tester.c for GCC 7 -Wstringop-overflow=.

    GCC 7 has a -Wstringop-overflow= warning that includes warning for
    strncat with a size specified that is larger than the size of the
    buffer (which is dubious usage, but valid at runtime if in fact there
    isn't an overflow with the particular buffer contents present).
    string/tester.c tests such cases; this patch arranges for this warning
    to be ignored around relevant strncat calls.

    Tested compilation for aarch64 (GCC mainline) with
    build-many-glibcs.py; did execution testing for x86_64 (GCC 5).

        * string/tester.c (test_strncat): Disable -Wstringop-overflow=
        around tests of strncat with large sizes.

    (cherry picked from commit 3ecd616cc1782210d09c9678ec1a48899f19145b)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=76d8cfc034101aeda25ec3c2b1d3b58be99a10b6

commit 76d8cfc034101aeda25ec3c2b1d3b58be99a10b6
Author: Joseph Myers <[hidden email]>
Date:   Wed Jan 4 23:32:14 2017 +0000

    Fix malloc/ tests for GCC 7 -Walloc-size-larger-than=.

    GCC 7 has a -Walloc-size-larger-than= warning for allocations of half
    the address space or more.  This causes errors building glibc tests
    that deliberately test failure of very large allocations.  This patch
    arranges for this warning to be ignored around the problematic
    function calls.

    Tested compilation for aarch64 (GCC mainline) with
    build-many-glibcs.py; did execution testing for x86_64 (GCC 5).

        * malloc/tst-malloc.c: Include <libc-internal.h>.
        (do_test): Disable -Walloc-size-larger-than= around tests of
        malloc with negative sizes.
        * malloc/tst-mcheck.c: Include <libc-internal.h>.
        (do_test): Disable -Walloc-size-larger-than= around tests of
        malloc and realloc with negative sizes.
        * malloc/tst-realloc.c: Include <libc-internal.h>.
        (do_test): Disable -Walloc-size-larger-than= around tests of
        realloc with negative sizes.

    (cherry picked from commit 3d7229c2507be1daf0c3e15e1f134076fa8b9025)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=45240ef5f46c231ef21216d2b681e4393c9658a9

commit 45240ef5f46c231ef21216d2b681e4393c9658a9
Author: Joseph Myers <[hidden email]>
Date:   Wed Jan 11 14:02:23 2017 +0000

    Fix testsuite build for GCC 7 -Wformat-truncation.

    This patch fixes the glibc testsuite build for GCC 7
    -Wformat-truncation, newly moved out of -Wformat-length and with some
    further warnings that didn't previously appear.  Two tests that
    previously disabled -Wformat-length are changed to disable
    -Wformat-truncation instead; two others are made to disable that
    option as well.

    Tested (compilation only) with build-many-glibcs.py for aarch64 with
    GCC mainline.

        * stdio-common/tst-printf.c [__GNUC_PREREQ (7, 0)]: Ignore
        -Wformat-truncation instead of -Wformat-length.
        * time/tst-strptime2.c (mkbuf) [__GNUC_PREREQ (7, 0)]: Likewise.
        * stdio-common/tstdiomisc.c (F): Ignore -Wformat-truncation for
        GCC 7.
        * wcsmbs/tst-wcstof.c: Include <libc-internal.h>.
        (do_test): Ignore -Wformat-truncation for GCC 7.

    (cherry picked from commit 3c9378265a8633e2c85a393b54a16abcf64fe616)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=ee5bfbe14c37667ed84d428e543a59765d9c8846

commit ee5bfbe14c37667ed84d428e543a59765d9c8846
Author: Steve Ellcey <[hidden email]>
Date:   Tue Nov 1 16:00:09 2016 -0700

    Fix warning from latest GCC in tst-printf.c

        * stdio-common/tst-printf.c: Ignore -Wformat-length warning.

    (cherry picked from commit 9032070deaa03431921315f973c548c2c403fecc)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=b343a6c400a449040d828fa17903e82818150c4d

commit b343a6c400a449040d828fa17903e82818150c4d
Author: Steve Ellcey <[hidden email]>
Date:   Wed Nov 2 16:00:39 2016 -0700

    Fix -Wformat-length warning in time/tst-strptime2.c

        * time/tst-strptime2.c: Ignore -Wformat-length warning.

    (cherry picked from commit 26d7185d6f0a79188fdf02c5eec6e52bb29112f8)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=a8d2af4a108dc4702cf7af55cd3e7c97fe608bf5

commit a8d2af4a108dc4702cf7af55cd3e7c97fe608bf5
Author: Joseph Myers <[hidden email]>
Date:   Wed Dec 21 23:44:01 2016 +0000

    Fix nss_nisplus build with mainline GCC (bug 20978).

    glibc build with current mainline GCC fails because
    nis/nss_nisplus/nisplus-alias.c contains code

      if (name != NULL)
        {
          *errnop = EINVAL;
          return NSS_STATUS_UNAVAIL;
        }

      char buf[strlen (name) + 9 + tablename_len];

    producing an error about strlen being called on a pointer that is
    always NULL (and a subsequent use of that pointer with a %s format in
    snprintf).

    As Andreas noted, the bogus conditional comes from a 1997 change:

    -  if (name == NULL || strlen(name) > 8)
    -    return NSS_STATUS_NOTFOUND;
    -  else
    +  if (name != NULL || strlen(name) <= 8)

    So the intention is clearly to return an error for NULL name.

    This patch duly inverts the sense of the conditional.  It fixes the
    build with GCC mainline, and passes usual glibc testsuite testing for
    x86_64.  However, I have not tried any actual substantive nisplus
    testing, do not have an environment for such testing, and do not know
    whether it is possible that strlen (name) or tablename_len might be
    large so that the VLA for buf is actually a security issue.  However,
    if it is a security issue, there are plenty of other similar instances
    in the nisplus code (that haven't been hidden by a bogus comparison
    with NULL) - and nis_table.c:__create_ib_request uses strdupa on the
    string passed to nis_list, so a local fix in the caller wouldn't
    suffice anyway (see bug 20987).  (Calls to strdupa and other such
    macros that use alloca must be considered equally questionable
    regarding stack overflow issues as direct calls to alloca and VLA
    declarations.)

        [BZ #20978]
        * nis/nss_nisplus/nisplus-alias.c (_nss_nisplus_getaliasbyname_r):
        Compare name == NULL, not name != NULL.

    (cherry picked from commit f88759ea9bd3c8d8fef28f123ba9767cb0e421a3)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=ee074bc17f6a9c81484e564cbacb55891390a78f

commit ee074bc17f6a9c81484e564cbacb55891390a78f
Author: Joseph Myers <[hidden email]>
Date:   Tue Nov 8 23:44:51 2016 +0000

    Fix rpcgen buffer overrun (bug 20790).

    Building with GCC 7 produces an error building rpcgen:

    rpc_parse.c: In function 'get_prog_declaration':
    rpc_parse.c:543:25: error: may write a terminating nul past the end of the
destination [-Werror=format-length=]
         sprintf (name, "%s%d", ARGNAME, num); /* default name of argument */
                         ~~~~^
    rpc_parse.c:543:5: note: format output between 5 and 14 bytes into a
destination of size 10
         sprintf (name, "%s%d", ARGNAME, num); /* default name of argument */
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

    That buffer overrun is for the case where the .x file declares a
    program with a million arguments.  The strcpy two lines above can
    generate a buffer overrun much more simply for a long argument name.

    The limit on length of line read by rpcgen (MAXLINESIZE == 1024)
    provides a bound on the buffer size needed, so this patch just changes
    the buffer size to MAXLINESIZE to avoid both possible buffer
    overruns.  A testcase is added that rpcgen does not crash with a
    500-character argument name, where it previously crashed.

    It would not at all surprise me if there are many other ways of
    crashing rpcgen with either valid or invalid input; fuzz testing would
    likely find various such bugs, though I don't think they are that
    important to fix (rpcgen is not that likely to be used with untrusted
    .x files as input).  (As well as fuzz-findable bugs there are probably
    also issues when various int variables get overflowed on very large
    input.)  The test infrastructure for rpcgen-not-crashing tests would
    need extending if tests are to be added for cases where rpcgen should
    produce an error, as opposed to cases where it should succeed.

    Tested for x86_64 and x86.

        [BZ #20790]
        * sunrpc/rpc_parse.c (get_prog_declaration): Increase buffer size
        to MAXLINESIZE.
        * sunrpc/bug20790.x: New file.
        * sunrpc/Makefile [$(run-built-tests) = yes] (rpcgen-tests): New
        variable.
        [$(run-built-tests) = yes] (tests-special): Add $(rpcgen-tests).
        [$(run-built-tests) = yes] ($(rpcgen-tests)): New rule.

    (cherry picked from commit 5874510faaf3cbd0bb112aaacab9f225002beed1)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=cfc025ebb627c50d4f777c64fb6584c02e291690

commit cfc025ebb627c50d4f777c64fb6584c02e291690
Author: Florian Weimer <[hidden email]>
Date:   Tue Jun 13 22:09:59 2017 +0200

    configure: Suppress expected compiler error message

    (cherry picked from commit c2528fef3b05bcffb1ac27c6c09cc3ff24b7f03f)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=67ca4485d7ab9384278d4104d18ece4f6c339c79

commit 67ca4485d7ab9384278d4104d18ece4f6c339c79
Author: H.J. Lu <[hidden email]>
Date:   Tue Jun 13 09:28:14 2017 -0700

    Make copy of <bits/std_abs.h> from GCC 7 [BZ #21573]

    <bits/std_abs.h> from GCC 7 will include /usr/include/stdlib.h from
    "#include_next" (instead of stdlib/stdlib.h in the glibc source
    directory), and this turns up as a make dependency.  Also make a copy
    of <bits/std_abs.h> to prevent it from including /usr/include/stdlib.h.

        [BZ #21573]
        * Makerules [$(c++-bits-std_abs-h) != ""] (before-compile): Add
        $(common-objpfx)bits/std_abs.h.
        [$(c++-bits-std_abs-h) != ""] ($(common-objpfx)bits/std_abs.h):
        New target.
        * config.make.in (c++-bits-std_abs-h): New.
        * configure.ac (find_cxx_header): Use "\,$1," with sed.
        (CXX_BITS_STD_ABS_H): New.
        (AC_SUBST(CXX_BITS_STD_ABS_H)): Likewise.
        * configure: Regenerated.

    (cherry picked from commit a65ea28d1833d3502c5070472e43bda04410e6b5)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=3387375fc8f737f44079f7a682501d4f0d090e60

commit 3387375fc8f737f44079f7a682501d4f0d090e60
Author: Florian Weimer <[hidden email]>
Date:   Wed Sep 21 10:45:32 2016 +0200

    Avoid running $(CXX) during build to obtain header file paths

    This reduces the build time somewhat and is particularly noticeable
    during rebuilds with few code changes.

    (cherry picked from commit fc3e1337be1c6935ab58bd13520f97a535cf70cc)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=f8e29d9269e985ec62383cd2b73f3715ed65afb2

commit f8e29d9269e985ec62383cd2b73f3715ed65afb2
Author: H.J. Lu <[hidden email]>
Date:   Thu Jun 29 10:26:04 2017 -0700

    i386: Increase MALLOC_ALIGNMENT to 16 [BZ #21120]

    GCC 7 changed the definition of max_align_t on i386:

   
https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=9b5c49ef97e63cc63f1ffa13baf771368105ebe2

    As a result, glibc malloc no longer returns memory blocks which are as
    aligned as max_align_t requires.

    This causes malloc/tst-malloc-thread-fail to fail with an error like this
    one:

    error: allocation function 0, size 144 not aligned to 16

    This patch increases the malloc alignment to 16 for i386.

        [BZ #21120]
        * sysdeps/generic/malloc-alignment.h: New file.
        * sysdeps/i386/malloc-alignment.h: Likewise.
        * sysdeps/generic/malloc-machine.h: Include <malloc-alignment.h>.

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=64a9f6a8acbbc0f7cd9c9532f5f16acefef15276

commit 64a9f6a8acbbc0f7cd9c9532f5f16acefef15276
Author: H.J. Lu <[hidden email]>
Date:   Fri Jun 23 14:38:46 2017 -0700

    Avoid .symver on common symbols [BZ #21666]

    The .symver directive on common symbol just creates a new common symbol,
    not an alias and the newer assembler with the bug fix for

    https://sourceware.org/bugzilla/show_bug.cgi?id=21661

    will issue an error.  Before the fix, we got

    $ readelf -sW libc.so | grep "loc[12s]"
      5109: 00000000003a0608     8 OBJECT  LOCAL  DEFAULT   36 loc1
      5188: 00000000003a0610     8 OBJECT  LOCAL  DEFAULT   36 loc2
      5455: 00000000003a0618     8 OBJECT  LOCAL  DEFAULT   36 locs
      6575: 00000000003a05f0     8 OBJECT  GLOBAL DEFAULT   36 locs@GLIBC_2.2.5
      7156: 00000000003a05f8     8 OBJECT  GLOBAL DEFAULT   36 loc1@GLIBC_2.2.5
      7312: 00000000003a0600     8 OBJECT  GLOBAL DEFAULT   36 loc2@GLIBC_2.2.5

    in libc.so.  The versioned loc1, loc2 and locs have the wrong addresses.
    After the fix, we got

    $ readelf -sW libc.so | grep "loc[12s]"
      6570: 000000000039e3b8     8 OBJECT  GLOBAL DEFAULT   34 locs@GLIBC_2.2.5
      7151: 000000000039e3c8     8 OBJECT  GLOBAL DEFAULT   34 loc1@GLIBC_2.2.5
      7307: 000000000039e3c0     8 OBJECT  GLOBAL DEFAULT   34 loc2@GLIBC_2.2.5

        [BZ #21666]
        * misc/regexp.c (loc1): Add __attribute__ ((nocommon));
        (loc2): Likewise.
        (locs): Likewise.

    (cherry picked from commit 388b4f1a02f3a801965028bbfcd48d905638b797)

-----------------------------------------------------------------------

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug network/20790] rpcgen buffer overrun in get_prog_declaration

glaubitz at physik dot fu-berlin.de
In reply to this post by glaubitz at physik dot fu-berlin.de
https://sourceware.org/bugzilla/show_bug.cgi?id=20790

--- Comment #6 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, hjl/pr21120/2.24 has been created
        at  646afc29797eed2aca4a42b3518b046e6c935e8b (commit)

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=646afc29797eed2aca4a42b3518b046e6c935e8b

commit 646afc29797eed2aca4a42b3518b046e6c935e8b
Author: H.J. Lu <[hidden email]>
Date:   Fri Jun 30 09:11:08 2017 -0700

    i386: Increase MALLOC_ALIGNMENT to 16 [BZ #21120]

    GCC 7 changed the definition of max_align_t on i386:

   
https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=9b5c49ef97e63cc63f1ffa13baf771368105ebe2

    As a result, glibc malloc no longer returns memory blocks which are as
    aligned as max_align_t requires.

    This causes malloc/tst-malloc-thread-fail to fail with an error like this
    one:

    error: allocation function 0, size 144 not aligned to 16

    This patch moves the MALLOC_ALIGNMENT definition to <malloc-alignment.h>
    and increases the malloc alignment to 16 for i386.

        [BZ #21120]
        * malloc/malloc.c (MALLOC_ALIGNMENT): Moved to ...
        * sysdeps/generic/malloc-alignment.h: Here.  New file.
        * sysdeps/i386/malloc-alignment.h: Likewise.
        * sysdeps/generic/malloc-machine.h: Include <malloc-alignment.h>.

    (cherry picked from commit 4e61a6be446026c327aa70cef221c9082bf0085d)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=dd3a46ce6722d29ea1a19bd8629a98b26d606ab7

commit dd3a46ce6722d29ea1a19bd8629a98b26d606ab7
Author: Stefan Liebler <[hidden email]>
Date:   Fri Feb 24 13:30:13 2017 +0100

    Get rid of duplicate const declaration specifier warning in
tst-resolv-qtypes.c.

    Compiling resolv/tst-resolv-qtypes.c with GCC 7 results in:
    tst-resolv-qtypes.c:53:14: error: duplicate ‘const’ declaration specifier
[-Werror=duplicate-decl-specifier]
     static const const char *domain = "www.example.com";

    This patch removes the duplicate const and makes domain a const pointer
    to const char literal.

    ChangeLog:

        * resolv/tst-resolv-qtypes.c (domain):
        Change type to const pointer to const char.

    (cherry picked from commit d4f94368a96541db2b38b6535402a941f5aff975)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=deaa1d0668b929692cfb4e33fbd1c0e9cac17305

commit deaa1d0668b929692cfb4e33fbd1c0e9cac17305
Author: Joseph Myers <[hidden email]>
Date:   Wed Jan 4 23:33:31 2017 +0000

    Fix string/tester.c for GCC 7 -Wstringop-overflow=.

    GCC 7 has a -Wstringop-overflow= warning that includes warning for
    strncat with a size specified that is larger than the size of the
    buffer (which is dubious usage, but valid at runtime if in fact there
    isn't an overflow with the particular buffer contents present).
    string/tester.c tests such cases; this patch arranges for this warning
    to be ignored around relevant strncat calls.

    Tested compilation for aarch64 (GCC mainline) with
    build-many-glibcs.py; did execution testing for x86_64 (GCC 5).

        * string/tester.c (test_strncat): Disable -Wstringop-overflow=
        around tests of strncat with large sizes.

    (cherry picked from commit 3ecd616cc1782210d09c9678ec1a48899f19145b)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=c25a5e52184395b71dcca0b09457f1369e44f53a

commit c25a5e52184395b71dcca0b09457f1369e44f53a
Author: Joseph Myers <[hidden email]>
Date:   Wed Jan 4 23:32:14 2017 +0000

    Fix malloc/ tests for GCC 7 -Walloc-size-larger-than=.

    GCC 7 has a -Walloc-size-larger-than= warning for allocations of half
    the address space or more.  This causes errors building glibc tests
    that deliberately test failure of very large allocations.  This patch
    arranges for this warning to be ignored around the problematic
    function calls.

    Tested compilation for aarch64 (GCC mainline) with
    build-many-glibcs.py; did execution testing for x86_64 (GCC 5).

        * malloc/tst-malloc.c: Include <libc-internal.h>.
        (do_test): Disable -Walloc-size-larger-than= around tests of
        malloc with negative sizes.
        * malloc/tst-mcheck.c: Include <libc-internal.h>.
        (do_test): Disable -Walloc-size-larger-than= around tests of
        malloc and realloc with negative sizes.
        * malloc/tst-realloc.c: Include <libc-internal.h>.
        (do_test): Disable -Walloc-size-larger-than= around tests of
        realloc with negative sizes.

    (cherry picked from commit 3d7229c2507be1daf0c3e15e1f134076fa8b9025)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=29a97495a36cb4af343c22faaebbe78644b436eb

commit 29a97495a36cb4af343c22faaebbe78644b436eb
Author: Joseph Myers <[hidden email]>
Date:   Wed Jan 11 14:02:23 2017 +0000

    Fix testsuite build for GCC 7 -Wformat-truncation.

    This patch fixes the glibc testsuite build for GCC 7
    -Wformat-truncation, newly moved out of -Wformat-length and with some
    further warnings that didn't previously appear.  Two tests that
    previously disabled -Wformat-length are changed to disable
    -Wformat-truncation instead; two others are made to disable that
    option as well.

    Tested (compilation only) with build-many-glibcs.py for aarch64 with
    GCC mainline.

        * stdio-common/tst-printf.c [__GNUC_PREREQ (7, 0)]: Ignore
        -Wformat-truncation instead of -Wformat-length.
        * time/tst-strptime2.c (mkbuf) [__GNUC_PREREQ (7, 0)]: Likewise.
        * stdio-common/tstdiomisc.c (F): Ignore -Wformat-truncation for
        GCC 7.
        * wcsmbs/tst-wcstof.c: Include <libc-internal.h>.
        (do_test): Ignore -Wformat-truncation for GCC 7.

    (cherry picked from commit 3c9378265a8633e2c85a393b54a16abcf64fe616)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=1b2acbfc1f457f745a9ea910602ee3e5fa2f6e91

commit 1b2acbfc1f457f745a9ea910602ee3e5fa2f6e91
Author: Steve Ellcey <[hidden email]>
Date:   Tue Nov 1 16:00:09 2016 -0700

    Fix warning from latest GCC in tst-printf.c

        * stdio-common/tst-printf.c: Ignore -Wformat-length warning.

    (cherry picked from commit 9032070deaa03431921315f973c548c2c403fecc)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=d248276e69af21c9729d8df6c920a984efccc6ae

commit d248276e69af21c9729d8df6c920a984efccc6ae
Author: Steve Ellcey <[hidden email]>
Date:   Wed Nov 2 16:00:39 2016 -0700

    Fix -Wformat-length warning in time/tst-strptime2.c

        * time/tst-strptime2.c: Ignore -Wformat-length warning.

    (cherry picked from commit 26d7185d6f0a79188fdf02c5eec6e52bb29112f8)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=67f1789e52e8b3a6107c536e160bc630f6b501e5

commit 67f1789e52e8b3a6107c536e160bc630f6b501e5
Author: Joseph Myers <[hidden email]>
Date:   Wed Dec 21 23:44:01 2016 +0000

    Fix nss_nisplus build with mainline GCC (bug 20978).

    glibc build with current mainline GCC fails because
    nis/nss_nisplus/nisplus-alias.c contains code

      if (name != NULL)
        {
          *errnop = EINVAL;
          return NSS_STATUS_UNAVAIL;
        }

      char buf[strlen (name) + 9 + tablename_len];

    producing an error about strlen being called on a pointer that is
    always NULL (and a subsequent use of that pointer with a %s format in
    snprintf).

    As Andreas noted, the bogus conditional comes from a 1997 change:

    -  if (name == NULL || strlen(name) > 8)
    -    return NSS_STATUS_NOTFOUND;
    -  else
    +  if (name != NULL || strlen(name) <= 8)

    So the intention is clearly to return an error for NULL name.

    This patch duly inverts the sense of the conditional.  It fixes the
    build with GCC mainline, and passes usual glibc testsuite testing for
    x86_64.  However, I have not tried any actual substantive nisplus
    testing, do not have an environment for such testing, and do not know
    whether it is possible that strlen (name) or tablename_len might be
    large so that the VLA for buf is actually a security issue.  However,
    if it is a security issue, there are plenty of other similar instances
    in the nisplus code (that haven't been hidden by a bogus comparison
    with NULL) - and nis_table.c:__create_ib_request uses strdupa on the
    string passed to nis_list, so a local fix in the caller wouldn't
    suffice anyway (see bug 20987).  (Calls to strdupa and other such
    macros that use alloca must be considered equally questionable
    regarding stack overflow issues as direct calls to alloca and VLA
    declarations.)

        [BZ #20978]
        * nis/nss_nisplus/nisplus-alias.c (_nss_nisplus_getaliasbyname_r):
        Compare name == NULL, not name != NULL.

    (cherry picked from commit f88759ea9bd3c8d8fef28f123ba9767cb0e421a3)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=92dfaf676f0adc88cbaa53a84d1a5947a37178a6

commit 92dfaf676f0adc88cbaa53a84d1a5947a37178a6
Author: Joseph Myers <[hidden email]>
Date:   Tue Nov 8 23:44:51 2016 +0000

    Fix rpcgen buffer overrun (bug 20790).

    Building with GCC 7 produces an error building rpcgen:

    rpc_parse.c: In function 'get_prog_declaration':
    rpc_parse.c:543:25: error: may write a terminating nul past the end of the
destination [-Werror=format-length=]
         sprintf (name, "%s%d", ARGNAME, num); /* default name of argument */
                         ~~~~^
    rpc_parse.c:543:5: note: format output between 5 and 14 bytes into a
destination of size 10
         sprintf (name, "%s%d", ARGNAME, num); /* default name of argument */
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

    That buffer overrun is for the case where the .x file declares a
    program with a million arguments.  The strcpy two lines above can
    generate a buffer overrun much more simply for a long argument name.

    The limit on length of line read by rpcgen (MAXLINESIZE == 1024)
    provides a bound on the buffer size needed, so this patch just changes
    the buffer size to MAXLINESIZE to avoid both possible buffer
    overruns.  A testcase is added that rpcgen does not crash with a
    500-character argument name, where it previously crashed.

    It would not at all surprise me if there are many other ways of
    crashing rpcgen with either valid or invalid input; fuzz testing would
    likely find various such bugs, though I don't think they are that
    important to fix (rpcgen is not that likely to be used with untrusted
    .x files as input).  (As well as fuzz-findable bugs there are probably
    also issues when various int variables get overflowed on very large
    input.)  The test infrastructure for rpcgen-not-crashing tests would
    need extending if tests are to be added for cases where rpcgen should
    produce an error, as opposed to cases where it should succeed.

    Tested for x86_64 and x86.

        [BZ #20790]
        * sunrpc/rpc_parse.c (get_prog_declaration): Increase buffer size
        to MAXLINESIZE.
        * sunrpc/bug20790.x: New file.
        * sunrpc/Makefile [$(run-built-tests) = yes] (rpcgen-tests): New
        variable.
        [$(run-built-tests) = yes] (tests-special): Add $(rpcgen-tests).
        [$(run-built-tests) = yes] ($(rpcgen-tests)): New rule.

    (cherry picked from commit 5874510faaf3cbd0bb112aaacab9f225002beed1)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=dc25211f1e94836f6c8e58bcd7e938feef4cf408

commit dc25211f1e94836f6c8e58bcd7e938feef4cf408
Author: Florian Weimer <[hidden email]>
Date:   Tue Jun 13 22:09:59 2017 +0200

    configure: Suppress expected compiler error message

    (cherry picked from commit c2528fef3b05bcffb1ac27c6c09cc3ff24b7f03f)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=4f990f7a50ddfbcad197a937de840937aad2fd44

commit 4f990f7a50ddfbcad197a937de840937aad2fd44
Author: H.J. Lu <[hidden email]>
Date:   Tue Jun 13 09:28:14 2017 -0700

    Make copy of <bits/std_abs.h> from GCC 7 [BZ #21573]

    <bits/std_abs.h> from GCC 7 will include /usr/include/stdlib.h from
    "#include_next" (instead of stdlib/stdlib.h in the glibc source
    directory), and this turns up as a make dependency.  Also make a copy
    of <bits/std_abs.h> to prevent it from including /usr/include/stdlib.h.

        [BZ #21573]
        * Makerules [$(c++-bits-std_abs-h) != ""] (before-compile): Add
        $(common-objpfx)bits/std_abs.h.
        [$(c++-bits-std_abs-h) != ""] ($(common-objpfx)bits/std_abs.h):
        New target.
        * config.make.in (c++-bits-std_abs-h): New.
        * configure.ac (find_cxx_header): Use "\,$1," with sed.
        (CXX_BITS_STD_ABS_H): New.
        (AC_SUBST(CXX_BITS_STD_ABS_H)): Likewise.
        * configure: Regenerated.

    (cherry picked from commit a65ea28d1833d3502c5070472e43bda04410e6b5)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=6184f4153fce2e77898c9a9c0f316ac19490fd6b

commit 6184f4153fce2e77898c9a9c0f316ac19490fd6b
Author: Florian Weimer <[hidden email]>
Date:   Wed Sep 21 10:45:32 2016 +0200

    Avoid running $(CXX) during build to obtain header file paths

    This reduces the build time somewhat and is particularly noticeable
    during rebuilds with few code changes.

    (cherry picked from commit fc3e1337be1c6935ab58bd13520f97a535cf70cc)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=64a9f6a8acbbc0f7cd9c9532f5f16acefef15276

commit 64a9f6a8acbbc0f7cd9c9532f5f16acefef15276
Author: H.J. Lu <[hidden email]>
Date:   Fri Jun 23 14:38:46 2017 -0700

    Avoid .symver on common symbols [BZ #21666]

    The .symver directive on common symbol just creates a new common symbol,
    not an alias and the newer assembler with the bug fix for

    https://sourceware.org/bugzilla/show_bug.cgi?id=21661

    will issue an error.  Before the fix, we got

    $ readelf -sW libc.so | grep "loc[12s]"
      5109: 00000000003a0608     8 OBJECT  LOCAL  DEFAULT   36 loc1
      5188: 00000000003a0610     8 OBJECT  LOCAL  DEFAULT   36 loc2
      5455: 00000000003a0618     8 OBJECT  LOCAL  DEFAULT   36 locs
      6575: 00000000003a05f0     8 OBJECT  GLOBAL DEFAULT   36 locs@GLIBC_2.2.5
      7156: 00000000003a05f8     8 OBJECT  GLOBAL DEFAULT   36 loc1@GLIBC_2.2.5
      7312: 00000000003a0600     8 OBJECT  GLOBAL DEFAULT   36 loc2@GLIBC_2.2.5

    in libc.so.  The versioned loc1, loc2 and locs have the wrong addresses.
    After the fix, we got

    $ readelf -sW libc.so | grep "loc[12s]"
      6570: 000000000039e3b8     8 OBJECT  GLOBAL DEFAULT   34 locs@GLIBC_2.2.5
      7151: 000000000039e3c8     8 OBJECT  GLOBAL DEFAULT   34 loc1@GLIBC_2.2.5
      7307: 000000000039e3c0     8 OBJECT  GLOBAL DEFAULT   34 loc2@GLIBC_2.2.5

        [BZ #21666]
        * misc/regexp.c (loc1): Add __attribute__ ((nocommon));
        (loc2): Likewise.
        (locs): Likewise.

    (cherry picked from commit 388b4f1a02f3a801965028bbfcd48d905638b797)

-----------------------------------------------------------------------

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug network/20790] rpcgen buffer overrun in get_prog_declaration

glaubitz at physik dot fu-berlin.de
In reply to this post by glaubitz at physik dot fu-berlin.de
https://sourceware.org/bugzilla/show_bug.cgi?id=20790

--- Comment #7 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, release/2.24/master has been updated
       via  7fca94796b67a8ca3730da255e64ee95a818f231 (commit)
      from  b3c498b834c112c10831dced77c9409cb79f339e (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=7fca94796b67a8ca3730da255e64ee95a818f231

commit 7fca94796b67a8ca3730da255e64ee95a818f231
Author: Joseph Myers <[hidden email]>
Date:   Sat Oct 7 13:42:41 2017 +0200

    Fix rpcgen buffer overrun (bug 20790).

    Building with GCC 7 produces an error building rpcgen:

    rpc_parse.c: In function 'get_prog_declaration':
    rpc_parse.c:543:25: error: may write a terminating nul past the end of the
destination [-Werror=format-length=]
         sprintf (name, "%s%d", ARGNAME, num); /* default name of argument */
                         ~~~~^
    rpc_parse.c:543:5: note: format output between 5 and 14 bytes into a
destination of size 10
         sprintf (name, "%s%d", ARGNAME, num); /* default name of argument */
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

    That buffer overrun is for the case where the .x file declares a
    program with a million arguments.  The strcpy two lines above can
    generate a buffer overrun much more simply for a long argument name.

    The limit on length of line read by rpcgen (MAXLINESIZE == 1024)
    provides a bound on the buffer size needed, so this patch just changes
    the buffer size to MAXLINESIZE to avoid both possible buffer
    overruns.  A testcase is added that rpcgen does not crash with a
    500-character argument name, where it previously crashed.

    It would not at all surprise me if there are many other ways of
    crashing rpcgen with either valid or invalid input; fuzz testing would
    likely find various such bugs, though I don't think they are that
    important to fix (rpcgen is not that likely to be used with untrusted
    .x files as input).  (As well as fuzz-findable bugs there are probably
    also issues when various int variables get overflowed on very large
    input.)  The test infrastructure for rpcgen-not-crashing tests would
    need extending if tests are to be added for cases where rpcgen should
    produce an error, as opposed to cases where it should succeed.

    Tested for x86_64 and x86.

        [BZ #20790]
        * sunrpc/rpc_parse.c (get_prog_declaration): Increase buffer size
        to MAXLINESIZE.
        * sunrpc/bug20790.x: New file.
        * sunrpc/Makefile [$(run-built-tests) = yes] (rpcgen-tests): New
        variable.
        [$(run-built-tests) = yes] (tests-special): Add $(rpcgen-tests).
        [$(run-built-tests) = yes] ($(rpcgen-tests)): New rule.

    (cherry picked from commit 5874510faaf3cbd0bb112aaacab9f225002beed1)

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog          |   11 +++++++++++
 NEWS               |    1 +
 sunrpc/Makefile    |   11 +++++++++++
 sunrpc/bug20790.x  |    1 +
 sunrpc/rpc_parse.c |    2 +-
 5 files changed, 25 insertions(+), 1 deletions(-)
 create mode 100644 sunrpc/bug20790.x

--
You are receiving this mail because:
You are on the CC list for the bug.
Reply | Threaded
Open this post in threaded view
|

[Bug network/20790] rpcgen buffer overrun in get_prog_declaration

Sourceware - glibc-bugs mailing list
In reply to this post by glaubitz at physik dot fu-berlin.de
https://sourceware.org/bugzilla/show_bug.cgi?id=20790

--- Comment #8 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Florian Weimer <[hidden email]>:

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=5500cdba4018ddbda7909bc7f4f9718610b43cf0

commit 5500cdba4018ddbda7909bc7f4f9718610b43cf0
Author: Petr Vorel <[hidden email]>
Date:   Mon Jul 13 19:36:35 2020 +0200

    Remove --enable-obsolete-rpc configure flag

    Sun RPC was removed from glibc. This includes rpcgen program, librpcsvc,
    and Sun RPC headers. Also test for bug #20790 was removed
    (test for rpcgen).

    Backward compatibility for old programs is kept only for architectures
    and ABIs that have been added in or before version 2.28.

    libtirpc is mature enough, librpcsvc and rpcgen are provided in
    rpcsvc-proto project.

    NOTE: libnsl code depends on Sun RPC (installed libnsl headers use
    installed Sun RPC headers), thus --enable-obsolete-rpc was a dependency
    for --enable-obsolete-nsl (removed in a previous commit).

    The arc ABI list file has to be updated because the port was added
    with the sunrpc symbols

    Tested-by: Carlos O'Donell <[hidden email]>
    Reviewed-by: Carlos O'Donell <[hidden email]>

--
You are receiving this mail because:
You are on the CC list for the bug.