|
 |
cc57d3 |
From 53e71571fc0b1f8dbad5f7ff6e9eeeb233496c13 Mon Sep 17 00:00:00 2001
|
|
|
cc57d3 |
From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= <ppisar@redhat.com>
|
|
|
cc57d3 |
Date: Thu, 13 Dec 2018 13:05:07 +0100
|
|
|
cc57d3 |
Subject: [PATCH] Fix a buffer overwrite in parse_stream()
|
|
|
cc57d3 |
|
|
|
cc57d3 |
The parse_stream() function allocates BUFSIZE-byte long output buffer. Then it
|
|
|
cc57d3 |
reads a string using PerlIO's read() with a maximal string length tsiz=BUFSIZE
|
|
|
cc57d3 |
characters into a temporary buffer. And then it retrieves a length of the string
|
|
|
cc57d3 |
in the temporary buffer in bytes and copies the strings from the temporary
|
|
|
cc57d3 |
buffer to the output buffer.
|
|
|
cc57d3 |
|
|
|
cc57d3 |
While it works for byte-stream file handles, when using UTF-8 handles, length
|
|
|
cc57d3 |
in bytes can be greater than length in characters, thus the temporary buffer
|
|
|
cc57d3 |
can contain more bytes than the size of the output buffer and we have a buffer
|
|
|
cc57d3 |
overwrite. This corrupts memory, especially metadata for libc memory
|
|
|
cc57d3 |
management and subsequent free() aborts with "free(): invalid next size
|
|
|
cc57d3 |
(normal)".
|
|
|
cc57d3 |
|
|
|
cc57d3 |
Minimal reproducer: Execute this code with an UTF-8 encoded file with non-ASCII
|
|
|
cc57d3 |
charcters on the standard input:
|
|
|
cc57d3 |
|
|
|
cc57d3 |
use XML::XPath;
|
|
|
cc57d3 |
use open ':std', ':encoding(UTF-8)';
|
|
|
cc57d3 |
my $xpath = XML::XPath->new(ioref => \*STDIN);
|
|
|
cc57d3 |
$xpath->find('/');
|
|
|
cc57d3 |
|
|
|
cc57d3 |
https://bugzilla.redhat.com/show_bug.cgi?id=1473368
|
|
|
cc57d3 |
https://bugzilla.redhat.com/show_bug.cgi?id=1658512
|
|
|
cc57d3 |
---
|
|
|
cc57d3 |
Expat/Expat.xs | 10 ++++++----
|
|
|
cc57d3 |
1 file changed, 6 insertions(+), 4 deletions(-)
|
|
|
cc57d3 |
|
|
|
cc57d3 |
diff --git a/Expat/Expat.xs b/Expat/Expat.xs
|
|
|
cc57d3 |
index ed66531..dbad380 100644
|
|
|
cc57d3 |
--- a/Expat/Expat.xs
|
|
|
cc57d3 |
+++ b/Expat/Expat.xs
|
|
|
cc57d3 |
@@ -343,8 +343,8 @@ parse_stream(XML_Parser parser, SV * ioref)
|
|
|
cc57d3 |
}
|
|
|
cc57d3 |
else {
|
|
|
cc57d3 |
tbuff = newSV(0);
|
|
|
cc57d3 |
- tsiz = newSViv(BUFSIZE);
|
|
|
cc57d3 |
- buffsize = BUFSIZE;
|
|
|
cc57d3 |
+ tsiz = newSViv(BUFSIZE); /* in UTF-8 characters */
|
|
|
cc57d3 |
+ buffsize = BUFSIZE * 6; /* in bytes that encode an UTF-8 string */
|
|
|
cc57d3 |
}
|
|
|
cc57d3 |
|
|
|
cc57d3 |
while (! done)
|
|
|
cc57d3 |
@@ -386,9 +386,11 @@ parse_stream(XML_Parser parser, SV * ioref)
|
|
|
cc57d3 |
croak("read error");
|
|
|
cc57d3 |
|
|
|
cc57d3 |
tb = SvPV(tbuff, br);
|
|
|
cc57d3 |
- if (br > 0)
|
|
|
cc57d3 |
+ if (br > 0) {
|
|
|
cc57d3 |
+ if (br > buffsize)
|
|
|
cc57d3 |
+ croak("The input buffer is not large enough for read UTF-8 decoded string");
|
|
|
cc57d3 |
Copy(tb, buffer, br, char);
|
|
|
cc57d3 |
- else
|
|
|
cc57d3 |
+ } else
|
|
|
cc57d3 |
done = 1;
|
|
|
cc57d3 |
|
|
|
cc57d3 |
PUTBACK ;
|
|
|
cc57d3 |
--
|
|
|
cc57d3 |
2.18.1
|
|
|
cc57d3 |
|