openssh unix dev - Mar 2015 - Invalid memory access / read stack overflow when reading config with zero bytes

On Mon, 30 Mar 2015 09:19:02 +1100 (AEDT)
Damien Miller <djm at mindrot.org> wrote:
> What version of OpenSSH is this?
6.8 portable on Linux.
> Also, when reporting fuzzer-derived problems it really helps to
> include the test-case.
The "test case" is a one byte file containing a zero byte. But here it
is :-)

-- 
Hanno B?ck
http://hboeck.de/

mail/jabber: hanno at hboeck.de
GPG: BBB51E42
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL:
<http://lists.mindrot.org/pipermail/openssh-unix-dev/attachments/20150330/9bfa2215/attachment.bin>

On Mon, 30 Mar 2015, Hanno B?ck wrote:
> On Mon, 30 Mar 2015 09:19:02 +1100 (AEDT)
> Damien Miller <djm at mindrot.org> wrote:
> 
> > What version of OpenSSH is this?
> 
> 6.8 portable on Linux.
That's strange - the line numbers in the valgrind stack trace don't
match. E.g.

==5578==    at 0x4C2CFCA: __GI_strchr (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==5578==    by 0x117B6B: process_config_line (readconf.c:785)
==5578==    by 0x119DED: read_config_file (readconf.c:1633)
> > Also, when reporting fuzzer-derived problems it really helps to
> > include the test-case.
> 
> The "test case" is a one byte file containing a zero byte. But
here it
> is :-)
Ok, I'll see if I can reproduce.

-d

On Sun, Mar 29, 2015 at 6:36 PM, Hanno B?ck <hanno at hboeck.de>
wrote:
> On Mon, 30 Mar 2015 09:19:02 +1100 (AEDT)
> Damien Miller <djm at mindrot.org> wrote:
>
>> What version of OpenSSH is this?
>
> 6.8 portable on Linux.
There are a *lot* of Linux flavors. Which one?
>> Also, when reporting fuzzer-derived problems it really helps to
>> include the test-case.
>
> The "test case" is a one byte file containing a zero byte. But
here it
> is :-)
>
> --
> Hanno B?ck
> http://hboeck.de/
>
> mail/jabber: hanno at hboeck.de
> GPG: BBB51E42
>
> _______________________________________________
> openssh-unix-dev mailing list
> openssh-unix-dev at mindrot.org
> https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
>

On Sun, 29 Mar 2015, Nico Kadel-Garcia wrote:
> On Sun, Mar 29, 2015 at 6:36 PM, Hanno B?ck <hanno at hboeck.de>
wrote:
> > On Mon, 30 Mar 2015 09:19:02 +1100 (AEDT)
> > Damien Miller <djm at mindrot.org> wrote:
> >
> >> What version of OpenSSH is this?
> >
> > 6.8 portable on Linux.
> 
> There are a *lot* of Linux flavors. Which one?
That doesn't matter much if he's using pristine sources.

-d

On Mon, 30 Mar 2015, Damien Miller wrote:
> On Mon, 30 Mar 2015, Hanno B?ck wrote:
> 
> > On Mon, 30 Mar 2015 09:19:02 +1100 (AEDT)
> > Damien Miller <djm at mindrot.org> wrote:
> > 
> > > What version of OpenSSH is this?
> > 
> > 6.8 portable on Linux.
> 
> That's strange - the line numbers in the valgrind stack trace don't
> match. E.g.
> 
> ==5578==    at 0x4C2CFCA: __GI_strchr (in
> /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==5578==    by 0x117B6B: process_config_line (readconf.c:785)
> ==5578==    by 0x119DED: read_config_file (readconf.c:1633)
reproduced; the line numbers were wrong.

diff --git a/readconf.c b/readconf.c
index 42a2961..5130407 100644
--- a/readconf.c
+++ b/readconf.c
@@ -763,7 +763,9 @@ process_config_line(Options *options, struct passwd *pw,
const char *host,
 	}
 
 	/* Strip trailing whitespace */
-	for (len = strlen(line) - 1; len > 0; len--) {
+	if ((len = strlen(line)) == 0)
+		return 0;
+	for (len--; len > 0; len--) {
 		if (strchr(WHITESPACE, line[len]) == NULL)
 			break;
 		line[len] = '\0';