Re: [Dev-luatex] [PATCH] printing.w: prevent OOB read from zero-length buffer

22 Apr 2016

On Fri, Apr 22, 2016 at 9:01 AM, luigi scarso 
wrote:
...
On Fri, Apr 22, 2016 at 8:14 AM, Philipp Gesang  wrote:
...
·········
...
On Thu, Apr 21, 2016 at 10:20 PM, Philipp Gesang 
wrote:
...
When passed the empty string, tprint() will allocate a zero-length
buffer and then branch on its first element (``if (*buffer) { … }``)
which has never been initialized.
Prevent the invalid access by checking for the empty string and
skipping
the printing routine entirely if appropriate.
Signed-off-by: Philipp Gesang 
---
 source/texk/web2c/luatexdir/tex/printing.w | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/source/texk/web2c/luatexdir/tex/printing.w
b/source/texk/web2c/luatexdir/tex/printing.w
index 478d55f..675fa45 100644
--- a/source/texk/web2c/luatexdir/tex/printing.w
+++ b/source/texk/web2c/luatexdir/tex/printing.w
@@ -367,6 +367,10 @@ void tprint(const char *sss)
     int newlinechar = int_par(new_line_char_code);
     int dolog = 0;
     int doterm = 0;
+    const size_t sss_len = strlen(sss);
+    if (sss_len == 0u) { /* nothing to print */
+        return;
+    }
     switch (selector) {
         case no_print:
             return;
@@ -413,7 +417,7 @@ void tprint(const char *sss)
     }
     /* what is left is the 3 term/log settings */
     if (dolog || doterm) {
-        buffer = xmalloc(strlen(sss)*3);
+        buffer = xmalloc(sss_len*3);
         if (dolog) {
             const unsigned char *ss = (const unsigned char *) sss;
             while (*ss) {
--
2.8.0
do you have an example that trigger the wrong sss string ?
For one it happens if the function is called on the empty string.
Here’s a test file:
a\bye
I use revision 5949. With the Plain loader from the pretest I
get:
$ valgrind --trace-children=yes mtxrun --script plain tprint-empty.tex
    …
    ==3758== Conditional jump or move depends on uninitialised value(s)
    ==3758==    at 0x4D77BE: tprint (printing.w:431)
    ==3758==    by 0x4D7A69: tprint_nl (printing.w:471)
    ==3758==    by 0x4BC19F: write_out (extensions.w:533)
    ==3758==    by 0x5B6BA4: out_what (pdflistout.w:262)
    ==3758==    by 0x4BBBAF: do_extension (extensions.w:389)
    ==3758==    by 0x4BBDF0: do_extension (extensions.w:418)
    ==3758==    by 0x4C2298: run_extension (maincontrol.w:616)
    ==3758==    by 0x4C43C2: main_control (maincontrol.w:971)
    ==3758==    by 0x4C0BAB: main_body (mainbody.w:461)
    ==3758==    by 0x4892B5: main (luatex.c:498)
Actually I just noticed that my patch is probably incorrect:
writeout invokes tprint_nl that way for the side effect of
popping a newline. The fix should thus be added further down
after that happened.
ok, thank you for the report.
--
luigi
fixed (not exactly this one, because it's not an issue) in trunk &  beta
0.95.0

-- 
luigi

    