Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CVE-2020-15366][1.x] Bump ajv from 4.11.8 to 6.12.6 #3769

Merged
merged 4 commits into from
May 15, 2023

Conversation

ananzh
Copy link
Member

@ananzh ananzh commented Apr 3, 2023

Description

An issue was discovered in ajv.validate() in ajv (aka Another JSON Schema Validator) 6.12.2. To fix this issue, the minimum required version is 6.12.3. In 1.x, there are two ajv versions, 6.12.6 and 4.11.8.

ubuntu@ip-172-31-55-237:~/work/OpenSearch-Dashboards$ yarn why ajv
yarn why v1.22.19
[1/4] Why do we have the module "ajv"...?
[2/4] Initialising dependency graph...
warning Resolution field "typescript@4.0.2" is incompatible with requested version "typescript@~4.8.4"
warning Resolution field "shelljs@0.8.5" is incompatible with requested version "shelljs@^0.6.0"
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "ajv@6.12.4"
info Has been hoisted to "ajv"
info Reasons this module exists
   - "workspace-aggregator-4bb3f374-9197-4114-85f8-bae14272a3f6" depends on it
   - Hoisted from "_project_#eslint#ajv"
   - Hoisted from "_project_#schema-utils#ajv"
   - Hoisted from "_project_#@osd#interpreter#webpack#ajv"
   - Hoisted from "_project_#request#har-validator#ajv"
   - Hoisted from "_project_#eslint#table#ajv"
   - Hoisted from "_project_#@osd#optimizer#istanbul-instrumenter-loader#schema-utils#ajv"
   - Hoisted from "_project_#@osd#ui-framework#postcss-loader#schema-utils#ajv"
   - Hoisted from "_project_#@osd#pm#string-replace-loader#schema-utils#ajv"
   - Hoisted from "_project_#@osd#ui-framework#webpack-dev-server#schema-utils#ajv"
   - Hoisted from "_project_#@osd#interpreter#webpack#schema-utils#ajv"
info Disk size without dependencies: "1.12MB"
info Disk size with unique dependencies: "1.99MB"
info Disk size with transitive dependencies: "2.04MB"
info Number of shared dependencies: 5
=> Found "@microsoft/tsdoc-config#ajv@6.12.6"
info This module exists because "_project_#@microsoft#api-extractor#@microsoft#tsdoc-config" depends on it.
=> Found "sass-lint#ajv@4.11.8"
info Reasons this module exists
   - "_project_#sass-lint#eslint#table" depends on it
   - Hoisted from "_project_#sass-lint#eslint#table#ajv"
=> Found "mini-css-extract-plugin#ajv@6.12.6"
info Reasons this module exists
   - "_project_#@osd#ui-shared-deps#mini-css-extract-plugin#schema-utils" depends on it
   - Hoisted from "_project_#@osd#ui-shared-deps#mini-css-extract-plugin#schema-utils#ajv"
=> Found "val-loader#ajv@6.12.6"
info Reasons this module exists
   - "_project_#@osd#ui-shared-deps#val-loader#schema-utils" depends on it
   - Hoisted from "_project_#@osd#ui-shared-deps#val-loader#schema-utils#ajv"
Done in 1.13s.

ajv @4.11.8 is brought from #sass-lint#eslint#table. There are breaking changes (V6.0.0, V5.0.0) between version 4 to 6.

  • JSON-Schema draft-06 support: Ajv v6 adds support for JSON-Schema draft-06. While it fully supports draft-04, some options and keywords have changed. This may require migration of your schemas or changes to your code that uses ajv.
  • Schema IDs: In v6, only the $id keyword is used as the schema ID by default. If you want to continue using the id keyword, you will need to set the schemaId option to id or auto.
  • Keyword changes: Some keywords have been added, removed, or changed. For example, the patternGroups keyword has been deprecated and removed, and new keywords like const, contains, and propertyNames have been added.
  • Formats: Some formats have been updated or added, such as the date, date-time, and time formats now supporting leap years and leap seconds.
  • Options: Some options have changed or have new default values. For example, the extendRefs option now defaults to ignore instead of true, and the unknownFormats option now defaults to true instead of ignore.
  • Asynchronous validation: The way asynchronous validation is handled has changed. For example, auto-detection of async mode and transpile option support now require the ajv-async package, and async schemas can only be compiled to async functions (compilation to generator functions is no longer supported).

However, ajv is only used directly by table for test purpose. In 1.x, we have table 3.8.3 and ajv is only used in 2 test files.

  • test/config.js
import {
    expect
} from 'chai';
import configSamples from './configSamples';
import validateConfig from '../dist/validateConfig';
import configSchema from '../src/schemas/config.json';
import Ajv from 'ajv';
import ajvKeywords from 'ajv-keywords';

describe('config.json schema', () => {
  var validate;

  before(() => {
    var ajv = new Ajv({allErrors: true});
    ajvKeywords(ajv, 'typeof');
    validate = ajv.compile(configSchema);
  });

  it('should pass validation of valid config samples', () => {
    configSamples.valid.forEach((sample, i) => {
      testValid(sample, validate);
      testValid(sample, validateConfig);
    });

    function testValid(sample, validate) {
      var valid = validate(sample);
      if (!valid) console.log(validate.errors);
      expect(valid).to.equal(true);
    }
  });

  it('should fail validation of invalid config samples', () => {
    configSamples.invalid.forEach((sample, i) => {
      testInvalid(sample, validate);
      testInvalid(sample, validateConfig);
    });

    function testInvalid(sample, validate) {
      var valid = validate(sample);
      expect(valid).to.equal(false);
    }
  });
});
  • test/streamConfig.js
import {
    expect
} from 'chai';
import configSamples from './streamConfigSamples';
import validateConfig from '../dist/validateStreamConfig';
import configSchema from '../src/schemas/streamConfig.json';
import Ajv from 'ajv';
import ajvKeywords from 'ajv-keywords';

describe('streamConfig.json schema', () => {
  var validate;

  before(() => {
    var ajv = new Ajv({allErrors: true});
    ajvKeywords(ajv, 'typeof');
    validate = ajv.compile(configSchema);
  });

  it('should pass validation of valid streamConfig samples', () => {
    configSamples.valid.forEach((sample, i) => {
      testValid(sample, validate);
      testValid(sample, validateConfig);
    });

    function testValid(sample, validate) {
      var valid = validate(sample);
      if (!valid) console.log(validate.errors);
      expect(valid).to.equal(true);
    }
  });

  it('should fail validation of invalid streamConfig samples', () => {
    configSamples.invalid.forEach((sample, i) => {
      testInvalid(sample, validate);
      testInvalid(sample, validateConfig);
    });

    function testInvalid(sample, validate) {
      var valid = validate(sample);
      expect(valid).to.equal(false);
    }
  });
});

Since the tests are part of the table package and are not part of OpenSearch Dashboards application, bumping ajv to version 6 should not have any direct impact on our application. We could also bump ajv to "ajv": "^6.12.6" in table v3.8.3. According to table's scripts section:

  "scripts": {
    "build": "rm -fr ./dist && babel --copy-files ./src --out-dir ./dist && npm run make-validators",
    "lint": "npm run build && eslint ./src ./tests",
    "make-readme": "gitdown ./.README/README.md --output-file ./README.md",
    "make-validators": "ajv compile --all-errors --inline-refs=false -s src/schemas/config -c ajv-keywords/keywords/typeof -o dist/validateConfig.js && ajv compile --all-errors --inline-refs=false -s src/schemas/streamConfig -c ajv-keywords/keywords/typeof -o dist/validateStreamConfig.js",
    "precommit": "npm run lint && npm run test",
    "prepublish": "NODE_ENV=production npm run build",
    "test": "npm run build && nyc --check-coverage mocha"
  },

We could run npm test :

  1) config.json schema "before all" hook:
     Error: no schema with key or ref "http://json-schema.org/draft-04/schema#"
      at Ajv.validate (node_modules/ajv/lib/ajv.js:93:19)
      at Ajv.validateSchema (node_modules/ajv/lib/ajv.js:174:20)
      at Ajv._addSchema (node_modules/ajv/lib/ajv.js:307:10)
      at Ajv.compile (node_modules/ajv/lib/ajv.js:113:24)
      at Context.<anonymous> (test/config.js:16:20)
      at processImmediate (internal/timers.js:464:21)

  2) streamConfig.json schema "before all" hook:
     Error: no schema with key or ref "http://json-schema.org/draft-04/schema#"
      at Ajv.validate (node_modules/ajv/lib/ajv.js:93:19)
      at Ajv.validateSchema (node_modules/ajv/lib/ajv.js:174:20)
      at Ajv._addSchema (node_modules/ajv/lib/ajv.js:307:10)
      at Ajv.compile (node_modules/ajv/lib/ajv.js:113:24)
      at Context.<anonymous> (test/streamConfig.js:16:20)
      at processImmediate (internal/timers.js:464:21)

We could also run npm run make-validators :

ubuntu@ip-172-31-55-237:~/work/packages/table$ npm run make-validators

> table@3.8.3 make-validators /home/ubuntu/work/packages/table
> ajv compile --all-errors --inline-refs=false -s src/schemas/config -c ajv-keywords/keywords/typeof -o dist/validateConfig.js && ajv compile --all-errors --inline-refs=false -s src/schemas/streamConfig -c ajv-keywords/keywords/typeof -o dist/validateStreamConfig.js

schema src/schemas/config is valid
schema src/schemas/streamConfig is valid

As shown, the ajv is used only for test and schema validation and only affects table package’s tests. Therefore, in OpenSearch Dashboards, we could add a resolution to bump ajv to 6.12.6 .

Issues Resolved

#1154

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
    • yarn test:ftr
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

Add a resolution to bump ajv from 4.11.8 to 6.12.6.

Issue Resolve
opensearch-project#1154

Signed-off-by: Anan Zhuang <ananzh@amazon.com>
@ananzh ananzh changed the title [CVE-2020-15366][1.x] Bump ajv from 4.7.0 to 6.12.6 [CVE-2020-15366][1.x] Bump ajv from 4.11.8 to 6.12.6 Apr 3, 2023
@ananzh ananzh requested a review from joshuarrrr April 3, 2023 17:54
@ananzh ananzh added cve Security vulnerabilities detected by Dependabot or Mend backport 1.3 v1.3.9 labels Apr 3, 2023
@codecov-commenter
Copy link

codecov-commenter commented Apr 3, 2023

Codecov Report

Merging #3769 (bd2214f) into 1.x (364832d) will decrease coverage by 0.05%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##              1.x    #3769      +/-   ##
==========================================
- Coverage   67.49%   67.45%   -0.05%     
==========================================
  Files        3044     3044              
  Lines       58692    58692              
  Branches     8902     8902              
==========================================
- Hits        39617    39588      -29     
- Misses      16926    16952      +26     
- Partials     2149     2152       +3     
Flag Coverage Δ
Linux ?
Windows 67.45% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 4 files with indirect coverage changes

@joshuarrrr joshuarrrr added v1.3.10 and removed v1.3.9 labels Apr 13, 2023
@AMoo-Miki
Copy link
Collaborator

AMoo-Miki commented Apr 15, 2023

ajv is only used directly by table for test purpose.

Looking at table@3.8.3, while visible usage of ajv is only in those tests, at build, it produces dist/validateStreamConfig.js and dist/validateConfig.js which are imported by files in its src. These 2 files have require('ajv/lib/compile/equal') in them which in ajv@4.11.8 is a custom implementation but in ajv@6.12.6 a library named fast-deep-equal is used. fast-deep-equal has some ugly edge-cases but considering all the above and the fact that sass-lint is just a dev-dep (and tests pass), I think it is safe resolve to ajv@6.12.6.

AMoo-Miki
AMoo-Miki previously approved these changes Apr 15, 2023
@joshuarrrr
Copy link
Member

All checks passed.

@joshuarrrr
Copy link
Member

@ananzh Converting to draft until you have a chance to address Miki's comments.

@joshuarrrr joshuarrrr marked this pull request as draft April 25, 2023 19:18
Signed-off-by: Josh Romero <rmerqg@amazon.com>
Copy link
Member

@kavilla kavilla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ISM still uses it and the new viz builder utilizes it but we should be conscience about this and not forward port this PR. But I don't think there is any issue with this and seems safe.

@abbyhu2000 abbyhu2000 merged commit 7786641 into opensearch-project:1.x May 15, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request May 15, 2023
Add a resolution to bump ajv from 4.11.8 to 6.12.6.

Issue Resolve
#1154

Signed-off-by: Anan Zhuang <ananzh@amazon.com>
Signed-off-by: Josh Romero <rmerqg@amazon.com>
Co-authored-by: Josh Romero <rmerqg@amazon.com>
(cherry picked from commit 7786641)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md
ananzh pushed a commit that referenced this pull request May 17, 2023
Add a resolution to bump ajv from 4.11.8 to 6.12.6.

Issue Resolve
#1154

Signed-off-by: Anan Zhuang <ananzh@amazon.com>
Signed-off-by: Josh Romero <rmerqg@amazon.com>
Co-authored-by: Josh Romero <rmerqg@amazon.com>
(cherry picked from commit 7786641)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.3 cve Security vulnerabilities detected by Dependabot or Mend v1.3.11
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants