diff options
author | Joseph Myers <joseph@codesourcery.com> | 2017-10-07 13:42:41 +0200 |
---|---|---|
committer | Florian Weimer <fweimer@redhat.com> | 2017-10-07 13:42:41 +0200 |
commit | 7fca94796b67a8ca3730da255e64ee95a818f231 (patch) | |
tree | 800487a97edf5384d6ab1acad8b5565948d5410f | |
parent | b3c498b834c112c10831dced77c9409cb79f339e (diff) | |
download | glibc-7fca94796b67a8ca3730da255e64ee95a818f231.tar.gz glibc-7fca94796b67a8ca3730da255e64ee95a818f231.tar.xz glibc-7fca94796b67a8ca3730da255e64ee95a818f231.zip |
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)
-rw-r--r-- | ChangeLog | 11 | ||||
-rw-r--r-- | NEWS | 1 | ||||
-rw-r--r-- | sunrpc/Makefile | 11 | ||||
-rw-r--r-- | sunrpc/bug20790.x | 1 | ||||
-rw-r--r-- | sunrpc/rpc_parse.c | 2 |
5 files changed, 25 insertions, 1 deletions
diff --git a/ChangeLog b/ChangeLog index 30dbdf1fd9..e593c33b0c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +2016-11-08 Joseph Myers <joseph@codesourcery.com> + + [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. + 2016-10-14 Steve Ellcey <sellcey@caviumnetworks.com> * sysdeps/ieee754/dbl-64/e_pow.c (checkint) Make conditions explicitly diff --git a/NEWS b/NEWS index f03910105a..e2935c063d 100644 --- a/NEWS +++ b/NEWS @@ -23,6 +23,7 @@ Security related changes: The following bugs are resolved with this release: + [20790] Fix rpcgen buffer overrun [21209] Ignore and remove LD_HWCAP_MASK for AT_SECURE programs [21289] Fix symbol redirect for fts_set [21386] Assertion in fork for distinct parent PID is incorrect diff --git a/sunrpc/Makefile b/sunrpc/Makefile index 4373fffdec..1e91905011 100644 --- a/sunrpc/Makefile +++ b/sunrpc/Makefile @@ -103,6 +103,11 @@ ifeq ($(have-thread-library),yes) xtests += thrsvc endif +ifeq ($(run-built-tests),yes) +rpcgen-tests := $(objpfx)bug20790.out +tests-special += $(rpcgen-tests) +endif + headers += $(rpcsvc:%.x=rpcsvc/%.h) extra-libs := librpcsvc extra-libs-others := librpcsvc # Make it in `others' pass, not `lib' pass. @@ -226,3 +231,9 @@ endif endif $(objpfx)thrsvc: $(common-objpfx)linkobj/libc.so $(shared-thread-library) + +ifeq ($(run-built-tests),yes) +$(rpcgen-tests): $(objpfx)%.out: %.x $(objpfx)rpcgen + $(built-program-cmd) -c $< -o $@; \ + $(evaluate-test) +endif diff --git a/sunrpc/bug20790.x b/sunrpc/bug20790.x new file mode 100644 index 0000000000..a00c9b3830 --- /dev/null +++ b/sunrpc/bug20790.x @@ -0,0 +1 @@ +program TPROG { version TVERS { int FUNC(int aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) = 1; } = 1; } = 1; diff --git a/sunrpc/rpc_parse.c b/sunrpc/rpc_parse.c index 1a1df6d8c2..505a6554cf 100644 --- a/sunrpc/rpc_parse.c +++ b/sunrpc/rpc_parse.c @@ -521,7 +521,7 @@ static void get_prog_declaration (declaration * dec, defkind dkind, int num /* arg number */ ) { token tok; - char name[10]; /* argument name */ + char name[MAXLINESIZE]; /* argument name */ if (dkind == DEF_PROGRAM) { |