All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] text-utils/ul: Fix buffer overflow
@ 2016-09-08 19:19 Tobias Stoeckmann
  2016-09-11 19:47 ` Shaun Tancheff
  0 siblings, 1 reply; 3+ messages in thread
From: Tobias Stoeckmann @ 2016-09-08 19:19 UTC (permalink / raw)
  To: util-linux

The text-utility ul can run into a buffer overflow on very long lines.
See this proof of concept how to reproduce the issue:

$ dd if=/dev/zero bs=1M count=10 | tr '\000' '\041' > poc.txt
$ echo -ne '\xe\x5f\x8\x5f\x61\x2\xf\x5f\x8\x5f' | dd of=poc.txt conv=notrunc
$ ul -i poc.txt > /dev/null # output would take ages
Segmentation fault
$ _

The problem manifests by using alloca with "maxcol", which can be as
large as INT_MAX, based on the input line.

A very long line (> 8 MB) with modes must be supplied to ul, as seen in
my proof of concept byte sequence above.

It is rather easy to fix this issue: allocate space on the heap instead.
maxcol could overflow here, but in that case no system will have enough
space to handle the request, properly ending ul through an err() call.


Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
---
 text-utils/ul.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/text-utils/ul.c b/text-utils/ul.c
index 6721974..3fd0b6a 100644
--- a/text-utils/ul.c
+++ b/text-utils/ul.c
@@ -402,11 +402,7 @@ static void flushln(void)
 static void overstrike(void)
 {
 	register int i;
-#ifdef __GNUC__
-	register wchar_t *lbuf = __builtin_alloca((maxcol + 1) * sizeof(wchar_t));
-#else
-	wchar_t lbuf[BUFSIZ];
-#endif
+	register wchar_t *lbuf = xmalloc((maxcol + 1) * sizeof(wchar_t));
 	register wchar_t *cp = lbuf;
 	int hadbold=0;
 
@@ -439,16 +435,13 @@ static void overstrike(void)
 		for (cp = lbuf; *cp; cp++)
 			putwchar(*cp == '_' ? ' ' : *cp);
 	}
+	free(lbuf);
 }
 
 static void iattr(void)
 {
 	register int i;
-#ifdef __GNUC__
-	register wchar_t *lbuf = __builtin_alloca((maxcol+1)*sizeof(wchar_t));
-#else
-	wchar_t lbuf[BUFSIZ];
-#endif
+	register wchar_t *lbuf = xmalloc((maxcol + 1) * sizeof(wchar_t));
 	register wchar_t *cp = lbuf;
 
 	for (i = 0; i < maxcol; i++)
@@ -465,6 +458,7 @@ static void iattr(void)
 		*cp = 0;
 	fputws(lbuf, stdout);
 	putwchar('\n');
+	free(lbuf);
 }
 
 static void initbuf(void)
-- 
2.10.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] text-utils/ul: Fix buffer overflow
  2016-09-08 19:19 [PATCH] text-utils/ul: Fix buffer overflow Tobias Stoeckmann
@ 2016-09-11 19:47 ` Shaun Tancheff
  2016-09-29  9:59   ` Karel Zak
  0 siblings, 1 reply; 3+ messages in thread
From: Shaun Tancheff @ 2016-09-11 19:47 UTC (permalink / raw)
  To: Tobias Stoeckmann; +Cc: util-linux

On Thu, Sep 8, 2016 at 2:19 PM, Tobias Stoeckmann <tobias@stoeckmann.org> w=
rote:
> The text-utility ul can run into a buffer overflow on very long lines.
> See this proof of concept how to reproduce the issue:
>
> $ dd if=3D/dev/zero bs=3D1M count=3D10 | tr '\000' '\041' > poc.txt
> $ echo -ne '\xe\x5f\x8\x5f\x61\x2\xf\x5f\x8\x5f' | dd of=3Dpoc.txt conv=
=3Dnotrunc
> $ ul -i poc.txt > /dev/null # output would take ages
> Segmentation fault
> $ _
>
> The problem manifests by using alloca with "maxcol", which can be as
> large as INT_MAX, based on the input line.
>
> A very long line (> 8 MB) with modes must be supplied to ul, as seen in
> my proof of concept byte sequence above.
>
> It is rather easy to fix this issue: allocate space on the heap instead.
> maxcol could overflow here, but in that case no system will have enough
> space to handle the request, properly ending ul through an err() call.
>
>
> Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
> ---
>  text-utils/ul.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)

Just a nit .. may as well change 'i' from an int to a long here as well.
That should cover the theoretical overflow for 64 bit machines.

> diff --git a/text-utils/ul.c b/text-utils/ul.c
> index 6721974..3fd0b6a 100644
> --- a/text-utils/ul.c
> +++ b/text-utils/ul.c
> @@ -402,11 +402,7 @@ static void flushln(void)
>  static void overstrike(void)
>  {
>         register int i;
> -#ifdef __GNUC__
> -       register wchar_t *lbuf =3D __builtin_alloca((maxcol + 1) * sizeof=
(wchar_t));
> -#else
> -       wchar_t lbuf[BUFSIZ];
> -#endif
> +       register wchar_t *lbuf =3D xmalloc((maxcol + 1) * sizeof(wchar_t)=
);
>         register wchar_t *cp =3D lbuf;
>         int hadbold=3D0;
>
> @@ -439,16 +435,13 @@ static void overstrike(void)
>                 for (cp =3D lbuf; *cp; cp++)
>                         putwchar(*cp =3D=3D '_' ? ' ' : *cp);
>         }
> +       free(lbuf);
>  }
>
>  static void iattr(void)
>  {
>         register int i;
> -#ifdef __GNUC__
> -       register wchar_t *lbuf =3D __builtin_alloca((maxcol+1)*sizeof(wch=
ar_t));
> -#else
> -       wchar_t lbuf[BUFSIZ];
> -#endif
> +       register wchar_t *lbuf =3D xmalloc((maxcol + 1) * sizeof(wchar_t)=
);
>         register wchar_t *cp =3D lbuf;
>
>         for (i =3D 0; i < maxcol; i++)
> @@ -465,6 +458,7 @@ static void iattr(void)
>                 *cp =3D 0;
>         fputws(lbuf, stdout);
>         putwchar('\n');
> +       free(lbuf);
>  }
>
>  static void initbuf(void)
> --
> 2.10.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe util-linux" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  https://urldefense.proofpoint.com/v2/url?u=3Dhttp=
-3A__vger.kernel.org_majordomo-2Dinfo.html&d=3DDQIBAg&c=3DIGDlg0lD0b-nebmJJ=
0Kp8A&r=3DWg5NqlNlVTT7Ugl8V50qIHLe856QW0qfG3WVYGOrWzA&m=3DbbRXLX-CYSIowTPqY=
28dCLlGaWC6MpdmP2mvIBLoArU&s=3DTc2CgUqDnP1yFVDSU30eZqiQqUOBot-jL8ovTKiAHyk&=
e=3D



--=20
Shaun Tancheff

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] text-utils/ul: Fix buffer overflow
  2016-09-11 19:47 ` Shaun Tancheff
@ 2016-09-29  9:59   ` Karel Zak
  0 siblings, 0 replies; 3+ messages in thread
From: Karel Zak @ 2016-09-29  9:59 UTC (permalink / raw)
  To: Shaun Tancheff; +Cc: Tobias Stoeckmann, util-linux

On Sun, Sep 11, 2016 at 02:47:22PM -0500, Shaun Tancheff wrote:
> On Thu, Sep 8, 2016 at 2:19 PM, Tobias Stoeckmann <tobias@stoeckmann.org> wrote:
> > The text-utility ul can run into a buffer overflow on very long lines.
> > See this proof of concept how to reproduce the issue:
> >
> > $ dd if=/dev/zero bs=1M count=10 | tr '\000' '\041' > poc.txt
> > $ echo -ne '\xe\x5f\x8\x5f\x61\x2\xf\x5f\x8\x5f' | dd of=poc.txt conv=notrunc
> > $ ul -i poc.txt > /dev/null # output would take ages
> > Segmentation fault
> > $ _
> >
> > The problem manifests by using alloca with "maxcol", which can be as
> > large as INT_MAX, based on the input line.
> >
> > A very long line (> 8 MB) with modes must be supplied to ul, as seen in
> > my proof of concept byte sequence above.
> >
> > It is rather easy to fix this issue: allocate space on the heap instead.
> > maxcol could overflow here, but in that case no system will have enough
> > space to handle the request, properly ending ul through an err() call.
> >
> >
> > Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
> > ---
> >  text-utils/ul.c | 14 ++++----------
> >  1 file changed, 4 insertions(+), 10 deletions(-)

Applied, thanks.

> 
> Just a nit .. may as well change 'i' from an int to a long here as well.
> That should cover the theoretical overflow for 64 bit machines.

This is generic problem of the 'ul' code, on many many places it uses 'int'
rather than 'size_t'. It would be better to fix it completely rather
than just one 'i' ;-)

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-09-29  9:59 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-08 19:19 [PATCH] text-utils/ul: Fix buffer overflow Tobias Stoeckmann
2016-09-11 19:47 ` Shaun Tancheff
2016-09-29  9:59   ` Karel Zak

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.