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

URI.js dependency security update #1297

Closed
solomchuk opened this issue Apr 6, 2022 · 3 comments · Fixed by #1300
Closed

URI.js dependency security update #1297

solomchuk opened this issue Apr 6, 2022 · 3 comments · Fixed by #1300
Assignees
Milestone

Comments

@solomchuk
Copy link

"medialize/uri.js": "1.19.0",

Our client recently performed a security scan on their instance of Skosmos, and flagged up (among other things) that the URI.js library version v1.19.0 has a known vulnerability, CVE-2021-3647. This is fixed in v1.19.7, and the latest is v1.19.11. In fact, most of the changes in URI.js between versions 1.19.0 and 1.19.11 are flagged as security fixes.

As a workaround for now, in our Docker builds we will be using a modified composer.json file with URI.js version set to 1.19.11. Would this version update be feasible in the baseline Skosmos repo?

@osma
Copy link
Member

osma commented Apr 8, 2022

Thanks for opening the issue @solomchuk ! You are right, we are depending on a very old version of URI.js.

I did some digging of the codebase trying to find out what we are using this library for, and the conclusion was a bit surprising - apparently it's not being used at all 😳 Although the script URI.min.js is loaded using a <script> tag, there are no uses of the class called URI that URI.js provides in the JS code of Skosmos.

The dependency was added a very long time, before we switched to Git and GitHub for version control in 2015, so I cannot trace back the commits related to this dependency. It's possible that it was used in the past in JS code, but then the calling code was rewritten to not use it anymore but we forgot to drop the dependency.

From a security perspective this is good news, since code that is not being called at all is unlikely to cause any security issues. In my understanding, and the security problems, at least the CVE you mention, are related to parsing of the URI string given to the class.

I briefly tested using Skosmos without the afore mentioned <script> tag and it seems to work just fine, so I will proceed with making a PR that removes the URI.js dependency entirely.

@osma osma added the bug label Apr 8, 2022
@osma osma added this to the 2.15 milestone Apr 8, 2022
@solomchuk
Copy link
Author

Thanks @osma, this is good news!

While looking for a workaround for this, I also found this note from URI.js maintainer:

IMPORTANT: You may not need URI.js anymore! Modern browsers provide the URL and URLSearchParams interfaces.

Perhaps this could be the reason you dropped the use of this library 🙂

osma added a commit that referenced this issue Apr 8, 2022
@osma osma mentioned this issue Apr 8, 2022
3 tasks
@osma osma closed this as completed in #1300 Apr 8, 2022
osma added a commit that referenced this issue Apr 8, 2022
(cherry picked from commit 1bfa01b)
@osma
Copy link
Member

osma commented Apr 8, 2022

Fixed in PR #1300 in the master branch and backported to the v2.14-maintenance branch in commit 2c55eef

@osma osma self-assigned this Apr 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants