Skip to contentSkip to conversation

text-utils/ul: Fix buffer overflow

PATCH Applied Tobias Stoeckmann 3 participants 3 messages Sep 8, 2016 – Sep 29, 2016 Util-Linux package

lore
Share & tools
lore

Tobias Stoeckmann

author
#0 lore raw

PATCH 1/1 text-utils/ul: Fix buffer overflow

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 <[email protected]>
---
 text-utils/ul.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)
text-utils/ul.c +4 -10
index 6721974..3fd0b6a 100644
@@ -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)
Signature · 1 line
2.10.0
#1 lore raw

On Thu, Sep 8, 2016 at 2:19 PM, Tobias Stoeckmann <[email protected]> w= rote:

Quoted 5 lines

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

Quoted 15 lines
$ 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.

Quoted 4 lines
Signed-off-by Tobias Stoeckmann <[email protected]>
---
 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.

text-utils/ul.c static void flushln(void) · line 406

Quoted 10 lines
text-utils/ul.c +0 -2
index 6721974..3fd0b6a 100644
@@ -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));

text-utils/ul.c static void flushln(void)

Quoted 4 lines
text-utils/ul.c +1 -3
-#else
-       wchar_t lbuf[BUFSIZ];
-#endif
+       register wchar_t *lbuf =3D xmalloc((maxcol + 1) * sizeof(wchar_t)=

);

Quoted 5 lines
text-utils/ul.c +0 -0
        register wchar_t *cp =3D lbuf;
        int hadbold=3D0;
 

@@ -439,16 +435,13 @@ static void overstrike(void)
                for (cp =3D lbuf; *cp; cp++)

text-utils/ul.c static void overstrike(void) · line 448

Quoted 10 lines
text-utils/ul.c +1 -2
                        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));

text-utils/ul.c static void flushln(void)

Quoted 4 lines
text-utils/ul.c +1 -3
-#else
-       wchar_t lbuf[BUFSIZ];
-#endif
+       register wchar_t *lbuf =3D xmalloc((maxcol + 1) * sizeof(wchar_t)=

);

Quoted 14 lines
text-utils/ul.c +1 -1
        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

Quoted 4 lines

--
To unsubscribe from this list: send the line "unsubscribe util-linux" in the body of a message to [email protected]
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

maintainer Applied
#2 lore raw
Quoted 21 lines · Shaun Tancheff
Quoted 20 lines · Tobias Stoeckmann

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.

Quoted 4 lines
Quoted 4 lines
Signed-off-by Tobias Stoeckmann <[email protected]>
---
 text-utils/ul.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

Applied, thanks.

Quoted 3 lines

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

Signature · 2 lines
 Karel Zak  <[email protected]>
 http://karelzak.blogspot.com