XSS (or parse error) in react-error-overlay with specific coding style

[ccloli]

Is this a bug report?

Yes

Can you also reproduce the problem with npm 4.x?

Reproduced on 5.0.3 and 4.6.1

Environment

1. node -v: v8.1.3
2. npm -v: 5.0.3 and 4.6.1
3. yarn --version (if you use Yarn):
4. npm ls react-scripts (if you haven’t ejected): react-scripts@1.0.10

Then, specify:

1. Operating system: Microsoft Windows 7 Professional x64 (6.1.7601)
2. Browser and version (if relevant): Google Chrome 59.0.3071.115 x64 (Stable)

Steps to Reproduce

1. create-react-app xss-test, then cd xss-test
2. Copy the files below and paste them to overwrite the files
3. npm start and open localhost:3000
4. Click any checkbox that are showing on page
5. An error dialog will shown, click the checkbox that shown on the source code panel
6. A dialog says /XSS/ will shown

Expected Behavior

It shouldn't show the checkbox, it should display the source code.

Actual Behavior

It shows the checkbox and the eval JavaScript of onclick attribute executed after clicked it.

Reproducible Demo

https://github.com/ccloli/create-react-app-xss-example

[gaearon]

That's great, thanks for reporting.

I expect this to be solved by the React rewrite (haven't reviewed it yet) but happy to take a fix for current code.

[ccloli]

Seems the source code is escaped by codeFrame() at line 44 of lib/components/code.js, then assigns it to variable ansiHighlight, then passes it to generateAnsiHtml() (react-dev-utils/ansiHTML)

This is the sourceCode.join('\n') that passed to codeFrame() and the parsed result ansiHighlight of the third section (no alert dialog):

// sourceCode
    `item-input-field ${
        data.checked ? 'checked-field' : ''
    }`.trim() }>
    <input type="checkbox" onclick="alert(/XSS/)" checked={ data.checked } onChange={ (event) => { this.onCheckboxChange(data.id); } } />
    {/* Should be `this.props.onCheckboxChange` */}
    { data.content }
</label>

// ansiHighlight
←[0m ←[90m 47 | ←[39m    ←[32m`item-input-field ${←[39m
 ←[90m 48 | ←[39m←[32m        data.checked ? 'checked-field' : ''←[39m
 ←[90m 49 | ←[39m←[32m    }`←[39m←[33m.←[39mtrim() }←[33m>←[39m
←[31m←[1m>←[22m←[39m←[90m 50 | ←[39m    ←[33m<←[39m←[33minput←[39m type←[33m=←[39m←[32m"checkbox"←[39m onclick←[33m=←[39m←[32m"alert(/XSS/)"←[39m checked←[33m=←[39m{ data←[33m.←[39mchecked } onChange←[33m=←[39m{ (event) ←[33m=>←[39m { ←[36mthis←[39m←[33m.←[39monCheckboxChange(data←[33m.←[39mid)←[33m;←[39m } } ←[33m/←[39m←[33m>←[39m
 ←[90m 51 | ←[39m    {←[90m/* Should be `this.props.onCheckboxChange` */←[39m}
 ←[90m 52 | ←[39m    { data←[33m.←[39mcontent }
 ←[90m 53 | ←[39m←[33m<←[39m←[33m/←[39m←[33mlabel←[39m←[33m>←[39m←[0m

And this is the first section (shows /XSS/ alert dialog):

// sourceCode
        data.checked ? 'checked-field' : ''
    }`.trim()
}>
    <input type="checkbox" onclick="alert(/XSS/)" checked={ data.checked } onChange={ (event) => { this.onCheckboxChange(data.id); } } />
    {/* Should be `this.props.onCheckboxChange` */}
    { data.content }
</label>

// ansiHighlight
←[0m ←[90m 10 | ←[39m        data←[33m.←[39mchecked ←[33m?←[39m ←[32m'checked-field'←[39m ←[33m:←[39m ←[32m''←[39m
 ←[90m 11 | ←[39m    }←[32m`.trim()←[39m
 ←[90m 12 | ←[39m←[32m}>←[39m
←[31m←[1m>←[22m←[39m←[90m 13 | ←[39m←[32m    <input type="checkbox" onclick="alert(/XSS/)" checked={ data.checked } onChange={ (event) => { this.onCheckboxChange(data.id); } } />←[39m
 ←[90m 14 | ←[39m←[32m    {/* Should be `←[39m←[36mthis←[39m←[33m.←[39mprops←[33m.←[39monCheckboxChange←[32m` */}←[39m
 ←[90m 15 | ←[39m←[32m    { data.content }←[39m
 ←[90m 16 | ←[39m←[32m</label>←[39m←[0m

It shows that the 4th line doesn't fully parsed by codeFrame() (thought seems that all the content of section is not fully parsed), and codeFrame is imported from babel-code-frame. So it's probably a bug of babel-code-frame...?

[ccloli]

Looks like the sourceCode is not fully parsed by codeFrame is because the sourceCode doesn't include the begin back-tick (`) of template literals, so it treats the latter tokens (begins from `.trim()) as a string. You can see that the screenshot of Expected Behavior parses the source code as JSX, while Actual Behavior is as string.

It maybe hard for babel-code-frame to fix this, because the source code is not complete, and the fragment of source code miss some important lexical items. At least we can fix the bug of parsing HTML tags in source code, simply replace the angle brackets in react-dev-utils/ansiHTML.