Insufficient input validation sendTransaction

[holiman]

System information

Version: `0.8.1-0.8.7`
OS & Version: all
Node type: `all`

Description

The the following payload tries to inject javascript into the elevated-privilege signature popup, by converting the payload to a RegExp:

var payload = 'document.body.innerHTML=\'Your accounts:\' +web3.personal.listAccounts.join(\'<li>\');return false;'
var xss = '</pre></div><button onclick="'+payload+'">AAA</button></div>'
var add = "0x0000000000000000000000000000000000000000";
web3.eth.sendTransaction({ from : add, to : add, data: new RegExp(xss)});

This attack fails, with the following stack-trace in the console:

  message: 'json: cannot unmarshal object into Go value of type string' }
[2016-10-28 09:24:00.306] [INFO] (ui: popup) - Exception in template helper: TypeError: this.data.replace is not a function
    at Object.formattedData (file:///data/tools/Mist-linux64-0-8-6/linux/resources/app.asar/interface/a9937a6cd93a5d79cd8de8f368032a8586fe93f1.js:283:3624)
    at file:///data/tools/Mist-linux64-0-8-6/linux/resources/app.asar/interface/a9937a6cd93a5d79cd8de8f368032a8586fe93f1.js:88:30879
...

Interestingly, the reason this attack fails is this line:

mist/interface/client/templates/popupWindows/sendTransactionConfirmation.js

Line 240 in c3ae760

: this.data.replace(/([0]{2,})/g,'<span class="zero">$1</span>');

    'formattedData': function(){
        return (TemplateVar.get('toIsContract'))
            ? this.data.replace(/([0]{2,})/g,'<span class="zero">$1</span>').replace(/(0x[a-f0-9]{8})/i,'<span class="function">$1</span>')
            : this.data.replace(/([0]{2,})/g,'<span class="zero">$1</span>');
    },

Basically, what happens is that the UI rendering tries to invoke the replace on a RegExp object, and fails.

The actual input validation is a bit flawed,

mist/modules/ipc/methods/eth_sendTransaction.js

Line 40 in c3ae760

if(_.isString(val)) {

:

                    if(_.isString(val)) {
                        if (val.match(/[^0-9a-fx]/igm)) {
                            throw this.ERRORS.INVALID_PAYLOAD;
                        }
                    }

Since the input is a RegExp, the isString returns false, and the intended validation that the input is hex-digits is bypassed.

This is very fragile, since a change to either the template used or the template-formatting function would open the signing window for XSS-attacks.

Suggested remediation

• Ensure that all input elements are expected type. Afaict, either number or string.

Example

x=/test/;
if(typeof(x)=='number'){ val=x;}
else{val=String(x)} 

[holiman]

Ping.. Any fixes in this area ?

[lock]

This thread has been automatically locked because it has not had recent activity. Please open a new issue for related bugs and link to relevant comments in this thread.