·········

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.

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