From d8e9cdc6bc202d598b2476517e863fab55e7f87d Mon Sep 17 00:00:00 2001 From: Ramesh Nair Date: Thu, 25 Aug 2016 16:43:11 +0800 Subject: [PATCH 1/2] filter returned account list even for batch calls --- .editorconfig | 9 +++ modules/ipc/ipcProviderBackend.js | 77 +++++++++++++++--- modules/ipc/ipcProviderWrapper.js | 2 +- modules/ipc/methods/base.js | 90 +++++++++------------- modules/ipc/methods/eth_accounts.js | 27 +++---- modules/ipc/methods/eth_compileSolidity.js | 13 +++- modules/ipc/methods/eth_sendTransaction.js | 13 ++++ 7 files changed, 153 insertions(+), 78 deletions(-) create mode 100644 .editorconfig diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 000000000..edb33caf8 --- /dev/null +++ b/.editorconfig @@ -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 \ No newline at end of file diff --git a/modules/ipc/ipcProviderBackend.js b/modules/ipc/ipcProviderBackend.js index 02dd3f563..f88e5ba1c 100644 --- a/modules/ipc/ipcProviderBackend.js +++ b/modules/ipc/ipcProviderBackend.js @@ -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."}, }; @@ -258,18 +259,76 @@ 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, nonErrorPayloads); + } 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, ret.result.shift()); + + 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); diff --git a/modules/ipc/ipcProviderWrapper.js b/modules/ipc/ipcProviderWrapper.js index 7120d786f..a9a8380de 100644 --- a/modules/ipc/ipcProviderWrapper.js +++ b/modules/ipc/ipcProviderWrapper.js @@ -26,7 +26,7 @@ ipc.on('ipcProvider-setWritable', function(e, writable){ -ipcProviderWrapper = { +const ipcProviderWrapper = { writable: false, /** diff --git a/modules/ipc/methods/base.js b/modules/ipc/methods/base.js index e875601f2..5148637e6 100644 --- a/modules/ipc/methods/base.js +++ b/modules/ipc/methods/base.js @@ -23,56 +23,14 @@ 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 []; - } - }) - .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; + return conn.socket.send(payload, { + fullResult: true, }); } @@ -88,16 +46,47 @@ module.exports = class BaseProcessor { /** - Sanitize a request or response payload. + Sanitize a request 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. + */ + sanitizeRequestPayload (conn, payload, isPartOfABatch) { + this._log.trace('Sanitize request payload', payload); + + this._sanitizeRequestResponsePayload(conn, payload, isPartOfABatch); + } + - This will modify the input payload object. + /** + 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. */ - sanitizePayload (conn, payload) { - this._log.trace('Sanitize payload', 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; } @@ -111,10 +100,7 @@ module.exports = class BaseProcessor { delete payload.result; payload.error = this.ERRORS.METHOD_DENIED; - } } - - }; diff --git a/modules/ipc/methods/eth_accounts.js b/modules/ipc/methods/eth_accounts.js index 78230e4f9..f565e1c1b 100644 --- a/modules/ipc/methods/eth_accounts.js +++ b/modules/ipc/methods/eth_accounts.js @@ -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); } } diff --git a/modules/ipc/methods/eth_compileSolidity.js b/modules/ipc/methods/eth_compileSolidity.js index 8b6cadfc1..b89bc6829 100644 --- a/modules/ipc/methods/eth_compileSolidity.js +++ b/modules/ipc/methods/eth_compileSolidity.js @@ -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 @@ -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); + } } diff --git a/modules/ipc/methods/eth_sendTransaction.js b/modules/ipc/methods/eth_sendTransaction.js index af2df8604..6d7544fc0 100644 --- a/modules/ipc/methods/eth_sendTransaction.js +++ b/modules/ipc/methods/eth_sendTransaction.js @@ -11,6 +11,19 @@ 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 */ From 3f328db516cae86931648782c89f2acaab340991 Mon Sep 17 00:00:00 2001 From: Ramesh Nair Date: Mon, 5 Sep 2016 16:23:58 +0800 Subject: [PATCH 2/2] more fixes| --- modules/ipc/ipcProviderBackend.js | 7 +++++-- modules/ipc/methods/base.js | 3 ++- modules/ipc/methods/eth_sendTransaction.js | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/modules/ipc/ipcProviderBackend.js b/modules/ipc/ipcProviderBackend.js index f88e5ba1c..78e4cfe68 100644 --- a/modules/ipc/ipcProviderBackend.js +++ b/modules/ipc/ipcProviderBackend.js @@ -289,7 +289,10 @@ class IpcProviderBackend { ? this._processors[finalPayload.method] : this._processors.base; - return processor.exec(conn, nonErrorPayloads); + return processor.exec( + conn, + isBatch ? nonErrorPayloads : nonErrorPayloads[0] + ); } else { return []; } @@ -304,7 +307,7 @@ class IpcProviderBackend { if (p.error) { finalResult.push(p); } else { - p = _.extend({}, p, ret.result.shift()); + p = _.extend({}, p, isBatch ? ret.shift() : ret); let processor = (this._processors[p.method]) ? this._processors[p.method] diff --git a/modules/ipc/methods/base.js b/modules/ipc/methods/base.js index 5148637e6..a9e351b7c 100644 --- a/modules/ipc/methods/base.js +++ b/modules/ipc/methods/base.js @@ -31,7 +31,8 @@ module.exports = class BaseProcessor { return conn.socket.send(payload, { fullResult: true, - }); + }) + .then((ret) => ret.result); } diff --git a/modules/ipc/methods/eth_sendTransaction.js b/modules/ipc/methods/eth_sendTransaction.js index 6d7544fc0..4966fe38f 100644 --- a/modules/ipc/methods/eth_sendTransaction.js +++ b/modules/ipc/methods/eth_sendTransaction.js @@ -30,7 +30,7 @@ module.exports = class extends BaseProcessor { exec (conn, payload) { return new Q((resolve, reject) => { this._log.info('Ask user for password'); - + this._log.info(payload.params[0]); // validate data