Merge pull request stellar#44 from stellar/decode-check-update

Add more checks to decodeCheck function

Author: bartekn

package.json

@@ -62,13 +62,13 @@
   },
   "dependencies": {
     "babel-runtime": "^4.7.16",
+    "base32.js": "~0.1.0",
     "crc": "^3.3.0",
     "ed25519": "0.0.4",
     "js-xdr": "0.0.11",
     "karma": "^0.13.9",
     "lodash": "^3.6.0",
     "sha.js": "^2.3.6",
-    "thirty-two": "^0.0.2",
     "tweetnacl": "^0.13.0"
   }
 }

src/account.js

@@ -10,7 +10,10 @@ export class Account {
     */
     static isValidAddress(address) {
         try {
-            decodeCheck("accountId", address);
+            let decoded = decodeCheck("accountId", address);
+            if (decoded.length !== 32) {
+                return false;
+            }
         } catch(err) {
             return false;
         }

src/keypair.js

@@ -38,6 +38,9 @@ export class Keypair {
 
   static fromAddress(address) {
     let publicKey = strkey.decodeCheck("accountId", address);
+    if (publicKey.length !== 32) {
+      throw new Error('Invalid Stellar address');
+    }
     return new this({publicKey});
   }
 

src/strkey.js

@@ -1,19 +1,27 @@
-import base32 from "thirty-two";
+import base32 from "base32.js";
 import crc from "crc";
-import {isUndefined, isNull} from "lodash";
+import {contains, isUndefined, isNull, isString} from "lodash";
 
 const versionBytes = {
   accountId: 0x30,
   seed:      0x90
 };
 
 export function decodeCheck(versionByteName, encoded) {
+  if (!isString(encoded)) {
+    throw new TypeError('encoded argument must be of type String');
+  }
+
   let decoded     = base32.decode(encoded);
   let versionByte = decoded[0];
   let payload     = decoded.slice(0, -2);
   let data        = payload.slice(1);
   let checksum    = decoded.slice(-2);
 
+  if (encoded != base32.encode(decoded)) {
+    throw new Error('invalid encoded string');
+  }
+
   let expectedVersion = versionBytes[versionByteName];
 
   if (isUndefined(expectedVersion)) {
@@ -30,10 +38,6 @@ export function decodeCheck(versionByteName, encoded) {
     throw new Error(`invalid checksum`);
   }
 
-  if (versionByteName === 'accountId' && decoded.length !== 35) {
-    throw new Error(`Decoded address length is invalid. Expected 35, got ${decoded.length}`);
-  }
-
   return new Buffer(data);
 }
 
@@ -54,7 +58,7 @@ export function encodeCheck(versionByteName, data) {
   let checksum      = calculateChecksum(payload);
   let unencoded     = Buffer.concat([payload, checksum]);
 
-  return base32.encode(unencoded).toString();
+  return base32.encode(unencoded);
 }
 
 function calculateChecksum(payload) {

test/unit/account_test.js

@@ -23,6 +23,11 @@ describe('Account.isValidAddress', function() {
 
   it("returns false for invalid address", function() {
     var addresses = [
+      'GBPXX0A5N4JYPESHAADMQKBPWZWQDQ64ZV6ZL2S3LAGW4SY7NTCMWIVL',
+      'GCFZB6L25D26RQFDWSSBDEYQ32JHLRMTT44ZYE3DZQUTYOL7WY43PLBG++',
+      'GADE5QJ2TY7S5ZB65Q43DFGWYWCPHIYDJ2326KZGAGBN7AE5UY6JVDRRA',
+      'GB6OWYST45X57HCJY5XWOHDEBULB6XUROWPIKW77L5DSNANBEQGUPADT2',
+      'GB6OWYST45X57HCJY5XWOHDEBULB6XUROWPIKW77L5DSNANBEQGUPADT2T',
       'GDXIIZTKTLVYCBHURXL2UPMTYXOVNI7BRAEFQCP6EZCY4JLKY4VKFNLT',
       'gWRYUerEKuz53tstxEuR3NCkiQDcV4wzFHmvLnZmj7PUqxW2wt',
       'test',

test/unit/keypair_test.js

@@ -106,7 +106,13 @@ describe('Keypair.fromAddress', function() {
     expect(() => StellarBase.Keypair.fromAddress("hel0")).to.throw()
     expect(() => StellarBase.Keypair.fromAddress("masterpassphrasemasterpassphrase")).to.throw()
     expect(() => StellarBase.Keypair.fromAddress("sfyjodTxbwLtRToZvi6yQ1KnpZriwTJ7n6nrASFR6goRviCU3Ff")).to.throw()
+  });
 
+  it("throws an error if the address isn't 32 bytes", function() {
+    expect(() => StellarBase.Keypair.fromAddress("masterpassphrasemasterpassphrase")).to.throw()
+    expect(() => StellarBase.Keypair.fromAddress("masterpassphrasemasterpassphrase")).to.throw()
+    expect(() => StellarBase.Keypair.fromAddress(null)).to.throw()
+    expect(() => StellarBase.Keypair.fromAddress()).to.throw()
   });
 
 });

test/unit/strkey_test.js

@@ -21,6 +21,21 @@ describe('StellarBase#decodeCheck', function() {
     expect(() => StellarBase.decodeCheck("accountId", "SBGWKM3CD4IL47QN6X54N6Y33T3JDNVI6AIJ6CD5IM47HG3IG4O36XCU")).to.throw(/invalid version/);
   });
 
+  it("throws an error when decoded data encodes to other string", function() {
+    // accountId
+    expect(() => StellarBase.decodeCheck("accountId", "GBPXX0A5N4JYPESHAADMQKBPWZWQDQ64ZV6ZL2S3LAGW4SY7NTCMWIVL")).to.throw(/invalid encoded string/);
+    expect(() => StellarBase.decodeCheck("accountId", "GCFZB6L25D26RQFDWSSBDEYQ32JHLRMTT44ZYE3DZQUTYOL7WY43PLBG++")).to.throw(/invalid encoded string/);
+    expect(() => StellarBase.decodeCheck("accountId", "GADE5QJ2TY7S5ZB65Q43DFGWYWCPHIYDJ2326KZGAGBN7AE5UY6JVDRRA")).to.throw(/invalid encoded string/);
+    expect(() => StellarBase.decodeCheck("accountId", "GB6OWYST45X57HCJY5XWOHDEBULB6XUROWPIKW77L5DSNANBEQGUPADT2")).to.throw(/invalid encoded string/);
+    expect(() => StellarBase.decodeCheck("accountId", "GB6OWYST45X57HCJY5XWOHDEBULB6XUROWPIKW77L5DSNANBEQGUPADT2T")).to.throw(/invalid encoded string/);
+    // seed
+    expect(() => StellarBase.decodeCheck("seed", "SB7OJNF5727F3RJUG5ASQJ3LUM44ELLNKW35ZZQDHMVUUQNGYW")).to.throw(/invalid encoded string/);
+    expect(() => StellarBase.decodeCheck("seed", "SB7OJNF5727F3RJUG5ASQJ3LUM44ELLNKW35ZZQDHMVUUQNGYWMEGB2W2")).to.throw(/invalid encoded string/);
+    expect(() => StellarBase.decodeCheck("seed", "SB7OJNF5727F3RJUG5ASQJ3LUM44ELLNKW35ZZQDHMVUUQNGYWMEGB2W2T")).to.throw(/invalid encoded string/);
+    expect(() => StellarBase.decodeCheck("seed", "SCMB30FQCIQAWZ4WQTS6SVK37LGMAFJGXOZIHTH2PY6EXLP37G46H6DT")).to.throw(/invalid encoded string/);
+    expect(() => StellarBase.decodeCheck("seed", "SAYC2LQ322EEHZYWNSKBEW6N66IRTDREEBUXXU5HPVZGMAXKLIZNM45H++")).to.throw(/invalid encoded string/);
+  });
+
   it("throws an error when the checksum is wrong", function() {
     expect(() => StellarBase.decodeCheck("accountId", "GBPXXOA5N4JYPESHAADMQKBPWZWQDQ64ZV6ZL2S3LAGW4SY7NTCMWIVT")).to.throw(/invalid checksum/);
     expect(() => StellarBase.decodeCheck("seed", "SBGWKM3CD4IL47QN6X54N6Y33T3JDNVI6AIJ6CD5IM47HG3IG4O36XCX")).to.throw(/invalid checksum/);