Merge pull request #614 from MetaMask/i546-NegativeErr

Add negative value validation to tx approval

2 files changed, 107 insertions, 0 deletions

diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js
index e94db2dfd..83827ec76 100644
--- a/app/scripts/metamask-controller.js
+++ b/app/scripts/metamask-controller.js
@@ -199,6 +199,9 @@ module.exports = class MetamaskController {
     const idStore = this.idStore
     var state = idStore.getState()
 
+    let err = this.enforceTxValidations(txParams)
+    if (err) return onTxDoneCb(err)
+
     // It's locked
     if (!state.isUnlocked) {
 
@@ -216,6 +219,13 @@ module.exports = class MetamaskController {
     }
   }
 
+  enforceTxValidations (txParams) {
+    if (txParams.value.indexOf('-') === 0) {
+      const msg = `Invalid transaction value of ${txParams.value} not a positive number.`
+      return new Error(msg)
+    }
+  }
+
   newUnsignedMessage (msgParams, cb) {
     var state = this.idStore.getState()
     if (!state.isUnlocked) {
diff --git a/test/unit/metamask-controller-test.js b/test/unit/metamask-controller-test.js
new file mode 100644
index 000000000..b87169ca2
--- /dev/null
+++ b/test/unit/metamask-controller-test.js
@@ -0,0 +1,97 @@
+var assert = require('assert')
+var MetaMaskController = require('../../app/scripts/metamask-controller')
+var sinon = require('sinon')
+var extend = require('xtend')
+const STORAGE_KEY = 'metamask-config'
+
+describe('MetaMaskController', function() {
+  const noop = () => {}
+  let controller = new MetaMaskController({
+    showUnconfirmedMessage: noop,
+    unlockAccountMessage: noop,
+    showUnconfirmedTx: noop,
+    setData,
+    loadData,
+  })
+
+  beforeEach(function() {
+    // sinon allows stubbing methods that are easily verified
+    this.sinon = sinon.sandbox.create()
+    window.localStorage = {} // Hacking localStorage support into JSDom
+  })
+
+  afterEach(function() {
+    // sinon requires cleanup otherwise it will overwrite context
+    this.sinon.restore()
+  })
+
+  describe('#enforceTxValidations', function () {
+    it('returns null for positive values', function() {
+      var sample = {
+        value: '0x01'
+      }
+      var res = controller.enforceTxValidations(sample)
+      assert.equal(res, null, 'no error')
+    })
+
+
+    it('returns error for negative values', function() {
+      var sample = {
+        value: '-0x01'
+      }
+      var res = controller.enforceTxValidations(sample)
+      assert.ok(res, 'error')
+    })
+  })
+})
+
+
+function loadData () {
+  var oldData = getOldStyleData()
+  var newData
+  try {
+    newData = JSON.parse(window.localStorage[STORAGE_KEY])
+  } catch (e) {}
+
+  var data = extend({
+    meta: {
+      version: 0,
+    },
+    data: {
+      config: {
+        provider: {
+          type: 'testnet',
+        },
+      },
+    },
+  }, oldData || null, newData || null)
+  return data
+}
+
+function getOldStyleData () {
+  var config, wallet, seedWords
+
+  var result = {
+    meta: { version: 0 },
+    data: {},
+  }
+
+  try {
+    config = JSON.parse(window.localStorage['config'])
+    result.data.config = config
+  } catch (e) {}
+  try {
+    wallet = JSON.parse(window.localStorage['lightwallet'])
+    result.data.wallet = wallet
+  } catch (e) {}
+  try {
+    seedWords = window.localStorage['seedWords']
+    result.data.seedWords = seedWords
+  } catch (e) {}
+
+  return result
+}
+
+function setData (data) {
+  window.localStorage[STORAGE_KEY] = JSON.stringify(data)
+}