From a14e756db54e03faf5439ff21c1db8bcfb223423 Mon Sep 17 00:00:00 2001 From: FUJI Goro Date: Tue, 4 May 2021 21:12:18 +0900 Subject: [PATCH 1/6] make sure Decoder raises DecodeError or DataViewIndexOutOfBoundsError --- src/DecodeError.ts | 16 ++++++++++++++++ src/Decoder.ts | 21 +++++---------------- src/index.ts | 5 +++-- src/timestamp.ts | 3 ++- 4 files changed, 26 insertions(+), 19 deletions(-) create mode 100644 src/DecodeError.ts diff --git a/src/DecodeError.ts b/src/DecodeError.ts new file mode 100644 index 00000000..39d8e08f --- /dev/null +++ b/src/DecodeError.ts @@ -0,0 +1,16 @@ + +export class DecodeError extends Error { + constructor(message: string) { + super(message); + + // fix the prototype chain in a cross-platform way + const proto: typeof DecodeError.prototype = Object.create(DecodeError.prototype); + Object.setPrototypeOf(this, proto); + + Object.defineProperty(this, "name", { + configurable: true, + enumerable: false, + value: DecodeError.name, + }); + } +} diff --git a/src/Decoder.ts b/src/Decoder.ts index 71f5104a..39ca7a25 100644 --- a/src/Decoder.ts +++ b/src/Decoder.ts @@ -4,6 +4,7 @@ import { getInt64, getUint64 } from "./utils/int"; import { utf8DecodeJs, TEXT_DECODER_THRESHOLD, utf8DecodeTD } from "./utils/utf8"; import { createDataView, ensureUint8Array } from "./utils/typedArrays"; import { CachedKeyDecoder, KeyDecoder } from "./CachedKeyDecoder"; +import { DecodeError } from "./DecodeError"; const enum State { ARRAY, @@ -60,22 +61,6 @@ const DEFAULT_MAX_LENGTH = 0xffff_ffff; // uint32_max const sharedCachedKeyDecoder = new CachedKeyDecoder(); -export class DecodeError extends Error { - constructor(message: string) { - super(message); - - // fix the prototype chain in a cross-platform way - const proto: typeof DecodeError.prototype = Object.create(DecodeError.prototype); - Object.setPrototypeOf(this, proto); - - Object.defineProperty(this, "name", { - configurable: true, - enumerable: false, - value: DecodeError.name, - }); - } -} - export class Decoder { private totalPos = 0; private pos = 0; @@ -133,6 +118,10 @@ export class Decoder { return new RangeError(`Extra ${view.byteLength - pos} of ${view.byteLength} byte(s) found at buffer[${posToShow}]`); } + /** + * @throws {DecodeError} + * @throws {RangeError} + */ public decode(buffer: ArrayLike | BufferSource): unknown { this.reinitializeState(); this.setBuffer(buffer); diff --git a/src/index.ts b/src/index.ts index 0c71804e..28560f1b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -13,8 +13,9 @@ export { DecodeOptions }; import { decodeAsync, decodeArrayStream, decodeMultiStream, decodeStream } from "./decodeAsync"; export { decodeAsync, decodeArrayStream, decodeMultiStream, decodeStream }; -import { Decoder, DecodeError } from "./Decoder"; -export { Decoder, DecodeError }; +import { Decoder, DataViewIndexOutOfBoundsError } from "./Decoder"; +import { DecodeError } from "./DecodeError"; +export { Decoder, DecodeError, DataViewIndexOutOfBoundsError }; import { Encoder } from "./Encoder"; export { Encoder }; diff --git a/src/timestamp.ts b/src/timestamp.ts index 79ee1b96..e3fe0155 100644 --- a/src/timestamp.ts +++ b/src/timestamp.ts @@ -1,4 +1,5 @@ // https://github.com/msgpack/msgpack/blob/master/spec.md#timestamp-extension-type +import { DecodeError } from "./DecodeError"; import { getInt64, setInt64 } from "./utils/int"; export const EXT_TIMESTAMP = -1; @@ -91,7 +92,7 @@ export function decodeTimestampToTimeSpec(data: Uint8Array): TimeSpec { return { sec, nsec }; } default: - throw new Error(`Unrecognized data size for timestamp: ${data.length}`); + throw new DecodeError(`Unrecognized data size for timestamp (expected 4, 8, or 12): ${data.length}`); } } From 0559682db9e71c79b60fd49c4bba36dc1db9f550 Mon Sep 17 00:00:00 2001 From: FUJI Goro Date: Tue, 4 May 2021 21:13:14 +0900 Subject: [PATCH 2/6] add test script: test:fuzz --- .gitignore | 3 +++ package.json | 5 +++-- test/decode.jsfuzz.js | 30 ++++++++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 test/decode.jsfuzz.js diff --git a/.gitignore b/.gitignore index c67b64cb..a2b46560 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,6 @@ isolate-*.log # flamebearer flamegraph.html + +# jsfuzz +corpus/ diff --git a/package.json b/package.json index f4a31df8..ca61e987 100644 --- a/package.json +++ b/package.json @@ -20,9 +20,10 @@ "test:te": "TEXT_ENCODING=force mocha 'test/**/*.test.ts'", "test:dist:purejs": "TS_NODE_PROJECT=tsconfig.test-dist-es5-purejs.json npm run test:purejs -- --reporter=dot", "test:cover": "npm run cover:clean && npm-run-all 'test:cover:*' && npm run cover:report", - "test:cover:purejs": "npx nyc --no-clean npm run test:purejs", - "test:cover:te": "npx nyc --no-clean npm run test:te", + "test:cover:purejs": "npm exec nyc --no-clean npm run test:purejs", + "test:cover:te": "npm exec nyc --no-clean npm run test:te", "test:deno": "deno test test/deno_test.ts", + "test:fuzz": "npm exec -- jsfuzz@git+https://gitlab.com/gitlab-org/security-products/analyzers/fuzzers/jsfuzz.git --fuzzTime 5 --rss-limit-mb 256 --no-versifier test/decode.jsfuzz.js corpus", "cover:clean": "rimraf .nyc_output coverage/", "cover:report": "npx nyc report --reporter=text-summary --reporter=html --reporter=json", "test:browser": "karma start --single-run", diff --git a/test/decode.jsfuzz.js b/test/decode.jsfuzz.js new file mode 100644 index 00000000..a76a5f6f --- /dev/null +++ b/test/decode.jsfuzz.js @@ -0,0 +1,30 @@ +/* eslint-disable */ +const assert = require("assert"); +const { Decoder, encode, DecodeError, DataViewIndexOutOfBoundsError } = require("../dist/index.js"); + +/** + * @param {Buffer} bytes + * @returns {void} + */ +module.exports.fuzz = function fuzz(bytes) { + const decoder = new Decoder(); + try { + decoder.decode(bytes); + } catch (e) { + if (e instanceof DecodeError) { + // ok + } else if (e instanceof DataViewIndexOutOfBoundsError) { + // ok + } else { + throw e; + } + } + + // make sure the decoder instance is not broken + const object = { + foo: 1, + bar: 2, + baz: ["one", "two", "three"], + }; + assert.deepStrictEqual(decoder.decode(encode(object)), object); +} From aa8bc4c527495ec643f567a75b927f5f2df86a06 Mon Sep 17 00:00:00 2001 From: FUJI Goro Date: Tue, 4 May 2021 21:17:09 +0900 Subject: [PATCH 3/6] tweaks for jsfuzz options --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index ca61e987..029d2508 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,7 @@ "test:cover:purejs": "npm exec nyc --no-clean npm run test:purejs", "test:cover:te": "npm exec nyc --no-clean npm run test:te", "test:deno": "deno test test/deno_test.ts", - "test:fuzz": "npm exec -- jsfuzz@git+https://gitlab.com/gitlab-org/security-products/analyzers/fuzzers/jsfuzz.git --fuzzTime 5 --rss-limit-mb 256 --no-versifier test/decode.jsfuzz.js corpus", + "test:fuzz": "npm exec -- jsfuzz@git+https://gitlab.com/gitlab-org/security-products/analyzers/fuzzers/jsfuzz.git --fuzzTime 60 --no-versifier test/decode.jsfuzz.js corpus", "cover:clean": "rimraf .nyc_output coverage/", "cover:report": "npx nyc report --reporter=text-summary --reporter=html --reporter=json", "test:browser": "karma start --single-run", From c72ff262a79a77df070172f796292cd96662e447 Mon Sep 17 00:00:00 2001 From: FUJI Goro Date: Tue, 4 May 2021 21:35:49 +0900 Subject: [PATCH 4/6] add fuzz.yml for CI --- .github/workflows/fuzz.yml | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 .github/workflows/fuzz.yml diff --git a/.github/workflows/fuzz.yml b/.github/workflows/fuzz.yml new file mode 100644 index 00000000..eae6dbee --- /dev/null +++ b/.github/workflows/fuzz.yml @@ -0,0 +1,23 @@ +# https://gitlab.com/gitlab-org/security-products/analyzers/fuzzers/jsfuzz + +name: Fuzz + +on: + push: + branches: + - main + pull_request: + +jobs: + fuzzing: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v2 + - name: Setup Node.js + uses: actions/setup-node@v1 + with: + node-version: "16" + + - run: npm ci + - run: npm run test:fuzz From 3a7f26c9cf4e52a433a83b3f2f7a59bf3c48bc6a Mon Sep 17 00:00:00 2001 From: FUJI Goro Date: Tue, 4 May 2021 21:41:24 +0900 Subject: [PATCH 5/6] remove IE11 trick from decode.jsfuzz.js --- test/decode.jsfuzz.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/decode.jsfuzz.js b/test/decode.jsfuzz.js index a76a5f6f..8dbe634d 100644 --- a/test/decode.jsfuzz.js +++ b/test/decode.jsfuzz.js @@ -1,6 +1,6 @@ /* eslint-disable */ const assert = require("assert"); -const { Decoder, encode, DecodeError, DataViewIndexOutOfBoundsError } = require("../dist/index.js"); +const { Decoder, encode, DecodeError } = require("../dist/index.js"); /** * @param {Buffer} bytes @@ -13,7 +13,7 @@ module.exports.fuzz = function fuzz(bytes) { } catch (e) { if (e instanceof DecodeError) { // ok - } else if (e instanceof DataViewIndexOutOfBoundsError) { + } else if (e instanceof RangeError) { // ok } else { throw e; From ae09f4880d11fbcb8a88267b02a6f56927f863fc Mon Sep 17 00:00:00 2001 From: FUJI Goro Date: Tue, 4 May 2021 21:45:29 +0900 Subject: [PATCH 6/6] revert use of npm exec --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 029d2508..3d208202 100644 --- a/package.json +++ b/package.json @@ -20,8 +20,8 @@ "test:te": "TEXT_ENCODING=force mocha 'test/**/*.test.ts'", "test:dist:purejs": "TS_NODE_PROJECT=tsconfig.test-dist-es5-purejs.json npm run test:purejs -- --reporter=dot", "test:cover": "npm run cover:clean && npm-run-all 'test:cover:*' && npm run cover:report", - "test:cover:purejs": "npm exec nyc --no-clean npm run test:purejs", - "test:cover:te": "npm exec nyc --no-clean npm run test:te", + "test:cover:purejs": "npx nyc --no-clean npm run test:purejs", + "test:cover:te": "npx nyc --no-clean npm run test:te", "test:deno": "deno test test/deno_test.ts", "test:fuzz": "npm exec -- jsfuzz@git+https://gitlab.com/gitlab-org/security-products/analyzers/fuzzers/jsfuzz.git --fuzzTime 60 --no-versifier test/decode.jsfuzz.js corpus", "cover:clean": "rimraf .nyc_output coverage/",