* Fix rpcgen buffer overrun (bug 20790)
@ 2016-11-07 18:40 Joseph Myers
2016-11-08 8:37 ` Florian Weimer
0 siblings, 1 reply; 2+ messages in thread
From: Joseph Myers @ 2016-11-07 18:40 UTC (permalink / raw)
To: libc-alpha
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.
2016-11-07 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.
diff --git a/sunrpc/Makefile b/sunrpc/Makefile
index 789ef42..99e5c3c 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.
@@ -225,3 +230,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 0000000..a00c9b3
--- /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 1a1df6d..505a655 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)
{
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: Fix rpcgen buffer overrun (bug 20790)
2016-11-07 18:40 Fix rpcgen buffer overrun (bug 20790) Joseph Myers
@ 2016-11-08 8:37 ` Florian Weimer
0 siblings, 0 replies; 2+ messages in thread
From: Florian Weimer @ 2016-11-08 8:37 UTC (permalink / raw)
To: Joseph Myers; +Cc: libc-alpha
On 11/07/2016 07:40 PM, Joseph Myers wrote:
> 2016-11-07 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.
This looks okay to me, all things considering. (Fixed-length buffers
are against GNU coding standards, but sunrpc/* doesn't really follow
them anyway.)
Thanks,
Florian
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-11-08 8:37 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-07 18:40 Fix rpcgen buffer overrun (bug 20790) Joseph Myers
2016-11-08 8:37 ` Florian Weimer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).