Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

A heap-buffer-overflow vunnerability of wasm #6585

Closed
ghost opened this issue Jan 11, 2019 · 3 comments
Closed

A heap-buffer-overflow vunnerability of wasm #6585

ghost opened this issue Jan 11, 2019 · 3 comments

Comments

@ghost
Copy link

ghost commented Jan 11, 2019

This bug is already reported at hackerone and has been solved
It's just public here.

Vendor:eosio
Affected product:libraries/wasm-jit
Product Version: 4.1
Git commit hash:29799eae7bb919e480981e6abf110dfa75198f8d

Summary:

A heap overflow in the jit-wasm which could lead to code execution. In function WAST::lex(), when adding the pointer nextToken, there is no boundary check.

Description:

it seems that the bug happens at the WAST::lex

        const Uptr numLineStarts = nextLineStart - lineStarts;
        const Uptr numTokens = nextToken - tokens;
        lineStarts = (U32*)realloc(lineStarts,sizeof(U32) * numLineStarts);
        tokens = (Token*)realloc(tokens,sizeof(Token) * numTokens);

        // Create the LineInfo object that encapsulates the line start information.
        outLineInfo = new LineInfo {lineStarts,U32(numLineStarts)};

when realloc the linsStarts

/* oldmem size */ 
if (__builtin_expect (chunksize_nomask (oldp) <= 2 * SIZE_SZ, 0)
     || __builtin_expect (oldsize >= av->system_mem, 0))
   malloc_printerr ("realloc(): invalid old size");

because oldsize >= av->system_mem,it will trigger the error "realloc(): invalid old size"
but why it will trigger this error, lets find when the lineStarts is malloced.
I wrote a gdb script to test it, and found that at

         nextToken->begin = U32(nextChar - string);
         NFA::StateIndex terminalState = staticData.nfaMachine.feed(nextChar);
         if(terminalState != NFA::unmatchedCharacterTerminal)
         {
            nextToken->type = TokenType(NFA::maximumTerminalStateIndex - (NFA::StateIndex)terminalState);
            ++nextToken;
         }

nextToken is not limited, and when tring to ++nextToken, it can assess to the chunk next to it which is lineStarts. it will overwrite the metadata of lineStarts, so we will have a heap overflow here...

@spoonincode
Copy link
Contributor

Yep, fixed

@Ofirnir123
Copy link

Yep, fixed

Hi , Can you refer me to the commit of the fix please ?

@spoonincode
Copy link
Contributor

It doesn't even matter any more -- at this point we no longer use WAVM's wast parser which is what this defect affected.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants