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

Update dependency xmldom #540

Merged
merged 2 commits into from Dec 26, 2021
Merged

Update dependency xmldom #540

merged 2 commits into from Dec 26, 2021

Conversation

karfau
Copy link
Contributor

@karfau karfau commented Dec 25, 2021

Switching from package xmldom to @xmldom/xmldom, which resolves the security issue present in latest xmldom version 0.6.0:
GHSA-5fg8-2547-mr8q

The reason is that the maintainers were forced to switch to a scoped package since 0.7.0:
xmldom/xmldom#271

  • I used node 12 to run npm install.
  • I executed npm run test on my machine without failure
  • npm run build:types failed since @xmldom/xmldom ships with types. I will push a separate commit to fix that.
  • all other steps of npm run prepublishOnly work fine
  • I'm assuming this fixes vulnerability in jsonld #488

I'm one of the xmldom maintainers. Don't hesitate to ask me questions.

Changes in xmldom since 0.6.0 ## [0.8.0](https://github.com/xmldom/xmldom/compare/0.7.5...0.8.0)

Fixed

  • Normalize all line endings according to XML specs 1.0 and 1.1
    BREAKING CHANGE: Certain combination of line break characters are normalized to a single \n before parsing takes place and will no longer be preserved.
  • XMLSerializer: Preserve whitespace character references #284 / #310
    BREAKING CHANGE: If you relied on the not spec compliant preservation of literal \t, \n or \r in attribute values.
    To preserve those you will have to create XML that instead contains the correct numerical (or hexadecimal) equivalent (e.g. 	, 
, 
).
  • Drop deprecated exports DOMImplementation and XMLSerializer from lib/dom-parser.js #53 / #309
    BREAKING CHANGE: Use the one provided by the main package export.
  • dom: Remove all links as part of removeChild #343 / #355

Chore

  • ci: Restore latest tested node version to 16.x #325
  • ci: Split test and lint steps into jobs #111 / #304
  • Pinned and updated devDependencies

Thank you @marrus-sh, @victorandree, @mdierolf, @tsabbay, @fatihpense for your contributions

0.7.5

Commits

Fixes:

0.7.4

Commits

Fixes:

  • Restore ability to parse __prototype__ attributes #315
    Thank you @dsimsonOMF

0.7.3

Commits

Fixes:

  • Add doctype when parsing from string #277 / #301
  • Correct typo in error message #294
    Thank you @rrthomas

Refactor:

  • Improve exports & require statements, new main package entry #233

Docs:

  • Fix Stryker badge #298
  • Fix link to help-wanted issues #299

Chore:

  • Execute stryker:dry-run on branches #302
  • Fix stryker config #300
  • Split test and lint scripts #297
  • Switch to stryker dashboard owned by org #292

0.7.2

Commits

Fixes:

  • Types: Add index.d.ts to packaged files #288
    Thank you @forty

0.7.1

Commits

Fixes:

  • Types: Copy types from DefinitelyTyped #283
    Thank you @kachkaev

Chore:

  • package.json: remove author, maintainers, etc. #279

0.7.0

Commits

Due to #271 this version was published as

  • unscoped xmldom package to github (git tags 0.7.0 and 0.7.0+unscoped)
  • scoped @xmldom/xmldom package to npm (git tag 0.7.0+scoped)
    For more details look at #278

Fixes:

  • Security: Misinterpretation of malicious XML input CVE-2021-32796
  • Implement Document.getElementsByClassName as specified #213, thank you @ChALkeR
  • Inherit namespace prefix from parent when required #268
  • Handle whitespace in closing tags #267
  • Update DOMImplementation according to recent specs #210
    BREAKING CHANGE: Only if you "passed features to be marked as available as a constructor arguments" and expected it to "magically work".
  • No longer serializes any namespaces with an empty URI #244
    (related to #168 released in 0.6.0)
    BREAKING CHANGE: Only if you rely on "unsetting" a namespace prefix by setting it to an empty string
  • Set localName as part of Document.createElement #229, thank you @rrthomas

CI

  • We are now additionally running tests against node v16
  • Stryker tests on the master branch now run against node v14

Docs

  • Describe relations with and between specs: #211, #247

Switching from package `xmldom` to `@xmldom/xmldom`, which resolves the security issue present in latest xmldom version 0.6.0:
GHSA-5fg8-2547-mr8q

The reason is that the maintainers were forced to switch to a scoped package since 0.7.0:
 xmldom/xmldom#271

- I used node 12 to run `npm install`.
- I executed `npm run test` on my machine without failure
- `npm run build:types` failed since `@xmldom/xmldom` ships with types. I will push a separate commit to fix that.
- all other steps of `npm run prepublishOnly` work fine

I'm one of the xmldom maintainers. Don't hesitate to ask me questions.
`namepsaceUri` is only present on the `Element` type.
By using a type checker ts knows that we have the right type.
@karfau
Copy link
Contributor Author

karfau commented Dec 26, 2021

I'm assuming the npm publish of PR only works when a contributor (re)triggers the build.

@bourgeoa bourgeoa merged commit b19ca97 into linkeddata:main Dec 26, 2021
@bourgeoa
Copy link
Contributor

@karfau
You are right. It does work when I merge.
May be you should have made your PR to a rdflib branch ? I have not implemented the action workflow, If you have anything to propose to resolve that it would be nice.

Thanks for your PR.

@karfau
Copy link
Contributor Author

karfau commented Dec 26, 2021

@bourgeoa As far as I can see it has to do with the fact that external contributors can not have access to the same secrets as maintainers, otherwise it would be to easy to extract them.

So there is nothing to propose or change about that.
But if the maintainers want to be sure they can rerun the jobs, which gives it access to the secrets.

Thx for landing the PR and happy coding

@karfau karfau deleted the update-xmldom branch December 26, 2021 13:57
@karfau
Copy link
Contributor Author

karfau commented Jan 10, 2022

PS: Since all existing xmldom versions have security issues you might want to evaluate if it's reasonable to deprecate version using that dependency.
It depends on, if the XML can be provided/modified by a potential attacker aka if it has to be treated as insecure user input anywhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

vulnerability in jsonld
2 participants