Skip to content
This repository has been archived by the owner on Sep 5, 2020. It is now read-only.

Filter result of eth.accounts even for batch IPC calls #1114

Merged
merged 2 commits into from
Sep 5, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
root = true

[*]
indent_style = space
indent_size = 4
end_of_line = lf
charset = utf-8
trim_trailing_whitespace = true
insert_final_newline = true
80 changes: 71 additions & 9 deletions modules/ipc/ipcProviderBackend.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const ERRORS = {
METHOD_TIMEOUT: {"code": -32603, "message": "Request timed out for method \'__method__\'."},
TX_DENIED: {"code": -32603, "message": "Transaction denied"},
BATCH_TX_DENIED: {"code": -32603, "message": "Transactions denied, sendTransaction is not allowed in batch requests."},
BATCH_COMPILE_DENIED: {"code": -32603, "message": "Compilation denied, compileSolidity is not allowed in batch requests."},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wasn't necessary for the fix but I added it anyway. You can't do a contract compilation as part of a batch call because we don't support that.

};


Expand Down Expand Up @@ -258,18 +259,79 @@ class IpcProviderBackend {
// reparse original string (so that we don't modify input payload)
let finalPayload = JSON.parse(originalPayloadStr);

this._sanitizeRequestPayload(conn, finalPayload);
// is batch?
const isBatch = _.isArray(finalPayload),
finalPayloadList = isBatch ? finalPayload : [finalPayload];

// sanitize each and every request payload
_.each(finalPayloadList, (p) => {
let processor = (this._processors[p.method])
? this._processors[p.method]
: this._processors.base;

processor.sanitizeRequestPayload(conn, p, isBatch);
});

// if a single payload and has an erro then throw it
if (!_.isArray(finalPayload) && finalPayload.error) {
// if a single payload and has an error then throw it
if (!isBatch && finalPayload.error) {
throw finalPayload.error;
}

if (this._processors[finalPayload.method]) {
return this._processors[finalPayload.method].exec(conn, finalPayload);
} else {
return this._processors.base.exec(conn, finalPayload);
}

// get non-error payloads
const nonErrorPayloads = _.filter(finalPayloadList, (p) => (!p.error));

// execute non-error payloads
return Q.try(() => {
if (nonErrorPayloads.length) {
// if single payload check if we have special processor for it
// if not then use base generic processor
let processor = (this._processors[finalPayload.method])
? this._processors[finalPayload.method]
: this._processors.base;

return processor.exec(
conn,
isBatch ? nonErrorPayloads : nonErrorPayloads[0]
);
} else {
return [];
}
})
.then((ret) => {
log.trace('Got result', ret);

let finalResult = [];

// collate results
_.each(finalPayloadList, (p) => {
if (p.error) {
finalResult.push(p);
} else {
p = _.extend({}, p, isBatch ? ret.shift() : ret);

let processor = (this._processors[p.method])
? this._processors[p.method]
: this._processors.base;

// sanitize response payload
processor.sanitizeResponsePayload(conn, p, isBatch);

finalResult.push(p);
}
});

// extract single payload result
if (!isBatch) {
finalResult = finalResult.pop();

// check if it's an error
if (finalResult.error) {
throw finalResult.error;
}
}

return finalResult;
});
})
.then((result) => {
log.trace('Got result', result);
Expand Down
2 changes: 1 addition & 1 deletion modules/ipc/ipcProviderWrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ ipc.on('ipcProvider-setWritable', function(e, writable){



ipcProviderWrapper = {
const ipcProviderWrapper = {
writable: false,

/**
Expand Down
91 changes: 39 additions & 52 deletions modules/ipc/methods/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,57 +23,16 @@ module.exports = class BaseProcessor {
/**
* Execute given request.
* @param {Object} conn IPCProviderBackend connection data.
* @param {Object|Array} payload JSON payload object
* @param {Object|Array} payload payload
* @return {Promise}
*/
exec (conn, payload) {
this._log.trace('Execute request', payload);

const isBatch = _.isArray(payload);

const payloadList = isBatch ? payload : [payload];

// filter out payloads which already have an error
const finalPayload = _.filter(payloadList, (p) => {
return !p.error;
});

return Q.try(() => {
if (finalPayload.length) {
return conn.socket.send(finalPayload, {
fullResult: true,
});
} else {
return [];
}
return conn.socket.send(payload, {
fullResult: true,
})
.then((ret) => {
let result = [];

_.each(payloadList, (p) => {
if (p.error) {
result.push(p);
} else {
p = _.extend({}, p, ret.result.shift());

this.sanitizePayload(conn, p);

result.push(p);
}
});

// if single payload
if (!isBatch) {
result = result[0];

// throw error if found
if (result.error) {
throw result.error;
}
}

return result;
});
.then((ret) => ret.result);
}


Expand All @@ -88,16 +47,47 @@ module.exports = class BaseProcessor {


/**
Sanitize a request or response payload.
Sanitize a request payload.

This will modify the input payload object.
This may modify the input payload object.

@param {Object} conn The connection.
@param {Object} payload The request payload.
@param {Boolean} isPartOfABatch Whether it's part of a batch payload.
*/
sanitizePayload (conn, payload) {
this._log.trace('Sanitize payload', payload);
sanitizeRequestPayload (conn, payload, isPartOfABatch) {
this._log.trace('Sanitize request payload', payload);

this._sanitizeRequestResponsePayload(conn, payload, isPartOfABatch);
}


/**
Sanitize a response payload.

This may modify the input payload object.

@param {Object} conn The connection.
@param {Object} payload The request payload.
@param {Boolean} isPartOfABatch Whether it's part of a batch payload.
*/
sanitizeResponsePayload (conn, payload, isPartOfABatch) {
this._log.trace('Sanitize response payload', payload);

this._sanitizeRequestResponsePayload(conn, payload, isPartOfABatch);
}


/**
Sanitize a request or response payload.

This may modify the input payload object.

@param {Object} conn The connection.
@param {Object} payload The request payload.
@param {Boolean} isPartOfABatch Whether it's part of a batch payload.
*/
_sanitizeRequestResponsePayload (conn, payload, isPartOfABatch) {
if (!_.isObject(payload)) {
throw this.ERRORS.INVALID_PAYLOAD;
}
Expand All @@ -111,10 +101,7 @@ module.exports = class BaseProcessor {
delete payload.result;

payload.error = this.ERRORS.METHOD_DENIED;

}
}


};

27 changes: 12 additions & 15 deletions modules/ipc/methods/eth_accounts.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,24 +12,21 @@ module.exports = class extends BaseProcessor {
/**
* @override
*/
exec (conn, payload) {
return super.exec(conn, payload)
.then((ret) => {
this._log.trace('Got account list', ret.result);
sanitizeResponsePayload (conn, payload, isPartOfABatch) {
this._log.trace('Sanitize account list', payload.result);

// if not an admin connection then do a check
if (!this._isAdminConnection(conn)) {
let tab = db.getCollection('tabs').findOne({ webviewId: conn.id });
// if not an admin connection then do a check
if (!this._isAdminConnection(conn)) {
let tab = db.getCollection('tabs').findOne({ webviewId: conn.id });

if(_.get(tab, 'permissions.accounts')) {
ret.result = _.intersection(ret.result, tab.permissions.accounts);
} else {
ret.result = [];
}
if(_.get(tab, 'permissions.accounts')) {
payload.result = _.intersection(payload.result, tab.permissions.accounts);
} else {
payload.result = [];
}

return ret;
});
}

return super.sanitizeResponsePayload(conn, payload, isPartOfABatch);
}
}

Expand Down
13 changes: 12 additions & 1 deletion modules/ipc/methods/eth_compileSolidity.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ module.exports = class extends BaseProcessor {
*/
exec (conn, payload) {
return Q.try(() => {
this._log.info('Compile solidity');
this._log.debug('Compile solidity');

let output = solc.compile(payload.params[0], 1); // 1 activates the optimiser

Expand All @@ -39,6 +39,17 @@ module.exports = class extends BaseProcessor {
return finalResult;
});
}

/**
* @override
*/
sanitizeRequestPayload (conn, payload, isPartOfABatch) {
if (isPartOfABatch) {
throw this.ERRORS.BATCH_COMPILE_DENIED;
}

return super.sanitizeRequestPayload(conn, payload, isPartOfABatch);
}
}


15 changes: 14 additions & 1 deletion modules/ipc/methods/eth_sendTransaction.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,26 @@ const ipc = electron.ipcMain;
* Process method: eth_sendTransaction
*/
module.exports = class extends BaseProcessor {

/**
* @override
*/
sanitizeRequestPayload (conn, payload, isPartOfABatch) {
if (isPartOfABatch) {
throw this.ERRORS.BATCH_TX_DENIED;
}

return super.sanitizeRequestPayload(conn, payload, isPartOfABatch);
}


/**
* @override
*/
exec (conn, payload) {
return new Q((resolve, reject) => {
this._log.info('Ask user for password');

this._log.info(payload.params[0]);

// validate data
Expand Down