From 0b8ddc1ee18c6d52fbc72177e8c5e180f9106708 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 25 May 2017 11:27:27 +0200 Subject: Add schema validation and assertion, create ECSignature schema --- package.json | 2 ++ src/ts/0x.js.ts | 3 ++- src/ts/globals.d.ts | 4 ++++ src/ts/schemas/ec_signature_schema.ts | 10 ++++++++++ src/ts/utils/assert.ts | 11 ++++++++--- src/ts/utils/schema_validator.ts | 13 +++++++++++++ 6 files changed, 39 insertions(+), 4 deletions(-) create mode 100644 src/ts/schemas/ec_signature_schema.ts create mode 100644 src/ts/utils/schema_validator.ts diff --git a/package.json b/package.json index 275a57a93..f6740b268 100644 --- a/package.json +++ b/package.json @@ -33,6 +33,7 @@ "devDependencies": { "@types/bignumber.js": "^4.0.2", "@types/chai": "^3.5.2", + "@types/jsonschema": "^1.1.1", "@types/mocha": "^2.2.41", "@types/node": "^7.0.22", "awesome-typescript-loader": "^3.1.3", @@ -53,6 +54,7 @@ "dependencies": { "bignumber.js": "^4.0.2", "ethereumjs-util": "^5.1.1", + "jsonschema": "^1.1.1", "web3": "^0.19.0" } } diff --git a/src/ts/0x.js.ts b/src/ts/0x.js.ts index acbdd01e2..8de87e96e 100644 --- a/src/ts/0x.js.ts +++ b/src/ts/0x.js.ts @@ -1,6 +1,7 @@ import * as BigNumber from 'bignumber.js'; import * as ethUtil from 'ethereumjs-util'; import {assert} from './utils/assert'; +import {ECSignatureSchema} from './schemas/ec_signature_schema'; /** * Elliptic Curve signature @@ -18,7 +19,7 @@ export class ZeroEx { */ public static isValidSignature(data: string, signature: ECSignature, signer: ETHAddressHex): boolean { assert.isString('data', data); - assert.isObject('signature', signature); + assert.doesConformToSchema('signature', signature, ECSignatureSchema); assert.isETHAddressHex('signer', signer); const dataBuff = ethUtil.toBuffer(data); diff --git a/src/ts/globals.d.ts b/src/ts/globals.d.ts index 0f7391b39..659638a1e 100644 --- a/src/ts/globals.d.ts +++ b/src/ts/globals.d.ts @@ -2,6 +2,10 @@ declare type ETHPublicKey = string; declare type ETHAddressHex = string; declare type ETHAddressBuff = Buffer; +declare interface Schema { + id: string; +} + declare module 'ethereumjs-util' { const toBuffer: (data: string) => Buffer; const hashPersonalMessage: (msg: Buffer) => Buffer; diff --git a/src/ts/schemas/ec_signature_schema.ts b/src/ts/schemas/ec_signature_schema.ts new file mode 100644 index 000000000..e4249afc2 --- /dev/null +++ b/src/ts/schemas/ec_signature_schema.ts @@ -0,0 +1,10 @@ +export const ECSignatureSchema = { + id: '/ECSignature', + properties: { + v: {type: 'number'}, + r: {type: 'string'}, + s: {type: 'string'}, + }, + required: ['v', 'r', 's'], + type: 'object', +}; diff --git a/src/ts/utils/assert.ts b/src/ts/utils/assert.ts index a29ae922d..7fb51fbdc 100644 --- a/src/ts/utils/assert.ts +++ b/src/ts/utils/assert.ts @@ -1,6 +1,7 @@ import * as _ from 'lodash'; import * as BigNumber from 'bignumber.js'; import Web3 = require('web3'); +import {SchemaValidator} from './schema_validator'; export const assert = { isBigNumber(variableName: string, value: BigNumber.BigNumber) { @@ -14,12 +15,16 @@ export const assert = { const web3 = new Web3(); this.assert(web3.isAddress(value), this.typeAssertionMessage(variableName, 'ETHAddressHex', value)); }, - isObject(variableName: string, value: object) { - this.assert(_.isObject(value), this.typeAssertionMessage(variableName, 'object', value)); - }, isNumber(variableName: string, value: number) { this.assert(_.isFinite(value), this.typeAssertionMessage(variableName, 'number', value)); }, + doesConformToSchema(variableName: string, value: object, schema: Schema) { + const schemaValidator = new SchemaValidator(); + const validationResult = schemaValidator.validate(value, schema); + const hasValidationErrors = validationResult.errors.length > 0; + const assertMsg = `Expected ${variableName} to conform to schema ${schema.id}, encountered: $value`; + this.assert(!hasValidationErrors, assertMsg); + }, assert(condition: boolean, message: string) { if (!condition) { throw new Error(message); diff --git a/src/ts/utils/schema_validator.ts b/src/ts/utils/schema_validator.ts new file mode 100644 index 000000000..a0d6f1bee --- /dev/null +++ b/src/ts/utils/schema_validator.ts @@ -0,0 +1,13 @@ +import {Validator as V, ValidatorResult} from 'jsonschema'; +import {ECSignatureSchema} from '../schemas/ec_signature_schema'; + +export class SchemaValidator { + private v: V; + constructor() { + this.v = new V(); + this.v.addSchema(ECSignatureSchema, ECSignatureSchema.id); + } + public validate(instance: object, schema: Schema): ValidatorResult { + return this.v.validate(instance, schema); + } +} -- cgit v1.2.3 From fa559377c80a75ba4169cab0825e5808154ae72b Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 25 May 2017 12:06:38 +0200 Subject: Improve ECSignature schema to check signature parameters and that v is 27 or 28 --- src/ts/schemas/ec_signature_schema.ts | 16 +++++++++++++--- src/ts/utils/schema_validator.ts | 3 ++- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/ts/schemas/ec_signature_schema.ts b/src/ts/schemas/ec_signature_schema.ts index e4249afc2..94e58e53c 100644 --- a/src/ts/schemas/ec_signature_schema.ts +++ b/src/ts/schemas/ec_signature_schema.ts @@ -1,9 +1,19 @@ +export const ECSignatureParameter = { + id: '/ECSignatureParameter', + type: 'string', + pattern: '^0[xX][0-9A-Fa-f]{64}$', +}; + export const ECSignatureSchema = { id: '/ECSignature', properties: { - v: {type: 'number'}, - r: {type: 'string'}, - s: {type: 'string'}, + v: { + type: 'number', + minimum: 27, + maximum: 28, + }, + r: {$ref: '/ECSignatureParameter'}, + s: {$ref: '/ECSignatureParameter'}, }, required: ['v', 'r', 's'], type: 'object', diff --git a/src/ts/utils/schema_validator.ts b/src/ts/utils/schema_validator.ts index a0d6f1bee..1cbffee58 100644 --- a/src/ts/utils/schema_validator.ts +++ b/src/ts/utils/schema_validator.ts @@ -1,10 +1,11 @@ import {Validator as V, ValidatorResult} from 'jsonschema'; -import {ECSignatureSchema} from '../schemas/ec_signature_schema'; +import {ECSignatureSchema, ECSignatureParameter} from '../schemas/ec_signature_schema'; export class SchemaValidator { private v: V; constructor() { this.v = new V(); + this.v.addSchema(ECSignatureParameter, ECSignatureParameter.id); this.v.addSchema(ECSignatureSchema, ECSignatureSchema.id); } public validate(instance: object, schema: Schema): ValidatorResult { -- cgit v1.2.3 From ae47981669e4c471623f22f8ecee93709dfe47bd Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 25 May 2017 12:08:21 +0200 Subject: Pretty print passed in order and schema validation errors in thrown assertion --- src/ts/utils/assert.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/ts/utils/assert.ts b/src/ts/utils/assert.ts index 7fb51fbdc..972047118 100644 --- a/src/ts/utils/assert.ts +++ b/src/ts/utils/assert.ts @@ -22,8 +22,10 @@ export const assert = { const schemaValidator = new SchemaValidator(); const validationResult = schemaValidator.validate(value, schema); const hasValidationErrors = validationResult.errors.length > 0; - const assertMsg = `Expected ${variableName} to conform to schema ${schema.id}, encountered: $value`; - this.assert(!hasValidationErrors, assertMsg); + const msg = `Expected ${variableName} to conform to schema ${schema.id} +Encountered: ${JSON.stringify(value, null, '\t')} +Validation errors: ${validationResult.errors.join(', ')}`; + this.assert(!hasValidationErrors, msg); }, assert(condition: boolean, message: string) { if (!condition) { -- cgit v1.2.3 From bcfb1a699a8c4469649af721875d878290198899 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 25 May 2017 12:08:49 +0200 Subject: Fix malformed tests to check for thrown assertion --- test/0x.js.ts | 37 +++++++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/test/0x.js.ts b/test/0x.js.ts index 71126c7da..644bc63b2 100644 --- a/test/0x.js.ts +++ b/test/0x.js.ts @@ -13,15 +13,19 @@ describe('ZeroEx library', () => { s: '0x2d887fd3b17bfdce3481f10bea41f45ba9f709d39ce8325427b57afcfc994cee', }; const address = '0x9b2055d370f73ec7d8a03e965129118dc8f5bf83'; - describe('should return false for malformed signature', () => { + describe('should throw if passed a malformed signature', () => { it('malformed v', () => { const malformedSignature = { v: 34, r: signature.r, s: signature.s, }; - const isValid = ZeroEx.isValidSignature(data, malformedSignature, address); - expect(isValid).to.be.false; + try { + const isValid = ZeroEx.isValidSignature(data, malformedSignature, address); + throw new Error('isValidSignature should have thrown'); + } catch (err) { + // continue + } }); it('r lacks 0x prefix', () => { const malformedR = signature.r.replace('0x', ''); @@ -30,18 +34,27 @@ describe('ZeroEx library', () => { r: malformedR, s: signature.s, }; - const isValid = ZeroEx.isValidSignature(data, malformedSignature, address); - expect(isValid).to.be.false; + try { + const isValid = ZeroEx.isValidSignature(data, malformedSignature, address); + throw new Error('isValidSignature should have thrown'); + } catch (err) { + // continue + } }); it('r is too short', () => { const malformedR = signature.r.substr(10); const malformedSignature = { v: signature.v, r: malformedR, - s: signature.s, + s: signature.s.replace('0', 'z'), }; - const isValid = ZeroEx.isValidSignature(data, malformedSignature, address); - expect(isValid).to.be.false; + try { + const isValid = ZeroEx.isValidSignature(data, malformedSignature, address); + throw new Error('isValidSignature should have thrown'); + } catch (err) { + console.log(err); + // continue + } }); it('s is not hex', () => { const malformedS = signature.s.replace('0', 'z'); @@ -50,8 +63,12 @@ describe('ZeroEx library', () => { r: signature.r, s: malformedS, }; - const isValid = ZeroEx.isValidSignature(data, malformedSignature, address); - expect(isValid).to.be.false; + try { + const isValid = ZeroEx.isValidSignature(data, malformedSignature, address); + throw new Error('isValidSignature should have thrown'); + } catch (err) { + // continue + } }); }); it('should return false if the data doesn\'t pertain to the signature & address', () => { -- cgit v1.2.3 From a5550a0b585dfb80176082d57ca0bf3ca1e4d1f9 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 25 May 2017 12:09:50 +0200 Subject: rename v to validator for clarity --- src/ts/utils/schema_validator.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ts/utils/schema_validator.ts b/src/ts/utils/schema_validator.ts index 1cbffee58..bd2f97d2b 100644 --- a/src/ts/utils/schema_validator.ts +++ b/src/ts/utils/schema_validator.ts @@ -1,14 +1,14 @@ -import {Validator as V, ValidatorResult} from 'jsonschema'; +import {Validator, ValidatorResult} from 'jsonschema'; import {ECSignatureSchema, ECSignatureParameter} from '../schemas/ec_signature_schema'; export class SchemaValidator { - private v: V; + private validator: Validator; constructor() { - this.v = new V(); - this.v.addSchema(ECSignatureParameter, ECSignatureParameter.id); - this.v.addSchema(ECSignatureSchema, ECSignatureSchema.id); + this.validator = new Validator(); + this.validator.addSchema(ECSignatureParameter, ECSignatureParameter.id); + this.validator.addSchema(ECSignatureSchema, ECSignatureSchema.id); } public validate(instance: object, schema: Schema): ValidatorResult { - return this.v.validate(instance, schema); + return this.validator.validate(instance, schema); } } -- cgit v1.2.3 From 56dfe6b2ff4a5ee7f5dd6c721f2e1cc9d6358998 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 25 May 2017 12:10:08 +0200 Subject: remove stray console log --- test/0x.js.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/0x.js.ts b/test/0x.js.ts index 644bc63b2..4d0b03a2a 100644 --- a/test/0x.js.ts +++ b/test/0x.js.ts @@ -52,7 +52,6 @@ describe('ZeroEx library', () => { const isValid = ZeroEx.isValidSignature(data, malformedSignature, address); throw new Error('isValidSignature should have thrown'); } catch (err) { - console.log(err); // continue } }); -- cgit v1.2.3 From 2117b4e0c87df277f5e8df54e8df922bdf19c298 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 25 May 2017 12:23:13 +0200 Subject: Use expect.to.throw --- test/0x.js.ts | 28 ++++------------------------ 1 file changed, 4 insertions(+), 24 deletions(-) diff --git a/test/0x.js.ts b/test/0x.js.ts index 2c4471e0f..09266018b 100644 --- a/test/0x.js.ts +++ b/test/0x.js.ts @@ -21,12 +21,7 @@ describe('ZeroEx library', () => { r: signature.r, s: signature.s, }; - try { - const isValid = ZeroEx.isValidSignature(data, malformedSignature, address); - throw new Error('isValidSignature should have thrown'); - } catch (err) { - // continue - } + expect(() => ZeroEx.isValidSignature(data, malformedSignature, address)).to.throw(); }); it('r lacks 0x prefix', () => { const malformedR = signature.r.replace('0x', ''); @@ -35,12 +30,7 @@ describe('ZeroEx library', () => { r: malformedR, s: signature.s, }; - try { - const isValid = ZeroEx.isValidSignature(data, malformedSignature, address); - throw new Error('isValidSignature should have thrown'); - } catch (err) { - // continue - } + expect(() => ZeroEx.isValidSignature(data, malformedSignature, address)).to.throw(); }); it('r is too short', () => { const malformedR = signature.r.substr(10); @@ -49,12 +39,7 @@ describe('ZeroEx library', () => { r: malformedR, s: signature.s.replace('0', 'z'), }; - try { - const isValid = ZeroEx.isValidSignature(data, malformedSignature, address); - throw new Error('isValidSignature should have thrown'); - } catch (err) { - // continue - } + expect(() => ZeroEx.isValidSignature(data, malformedSignature, address)).to.throw(); }); it('s is not hex', () => { const malformedS = signature.s.replace('0', 'z'); @@ -63,12 +48,7 @@ describe('ZeroEx library', () => { r: signature.r, s: malformedS, }; - try { - const isValid = ZeroEx.isValidSignature(data, malformedSignature, address); - throw new Error('isValidSignature should have thrown'); - } catch (err) { - // continue - } + expect(() => ZeroEx.isValidSignature(data, malformedSignature, address)).to.throw(); }); }); it('should return false if the data doesn\'t pertain to the signature & address', () => { -- cgit v1.2.3