Skip to content

Conversation

@he8us
Copy link
Contributor

@he8us he8us commented Oct 15, 2018

Hello,

While the development of #859 I noticed that the front end dependencies were copied in this repository or loaded from raw.githubusercontent.com. IMHO, that's a bad idea because:

  • For the first case: more code to manage, and how to update?
  • For the second case, how to know if the external library updates and introduce a breaking change?

So here's my proposal to be able to install the front-end dependencies through NPM

@he8us
Copy link
Contributor Author

he8us commented Oct 15, 2018

Apparently I introduced a vulnerable dependency, but snyk doesn't let me see it.
And when I run npm audit no vulnerabilities are found.

@melvincarvalho
Copy link
Contributor

Good catch & thanks for the PR!

I think it makes sense to remove dependencies on raw.githubusercontent.com

Re snyk, I tried to check but got :

Unable to display this organisation
The organisation does not exist, or you do not have permissions to access it.

@kjetilk
Copy link
Member

kjetilk commented Oct 15, 2018

Yes, this seems like something we should fix. With what urgency do you thing, @RubenVerborgh ?

Copy link
Contributor

@RubenVerborgh RubenVerborgh left a comment

Choose a reason for hiding this comment

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

Good idea of serving from npm packages. But let's not copy—they will get out of date.
Instead, let's reuse the technique for serving solid-auth-client and mashlib (#792) through the routeResolvedFile in utils.

Copy link
Contributor

@RubenVerborgh RubenVerborgh left a comment

Choose a reason for hiding this comment

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

Thanks a lot, thumbs up from me!

@RubenVerborgh RubenVerborgh removed their assignment Oct 15, 2018
@RubenVerborgh
Copy link
Contributor

Was going to let @kjetilk and @megoth merge this, but I'll just put this in develop for now. We should just merge develop into the v4.2.0 branch soon.

@RubenVerborgh RubenVerborgh assigned RubenVerborgh and unassigned kjetilk and megoth Oct 15, 2018
@RubenVerborgh RubenVerborgh changed the title Let the installation of the front-end assets through NPM Server front-end assets from node_modules Oct 15, 2018
@RubenVerborgh RubenVerborgh changed the title Server front-end assets from node_modules Serve front-end assets from node_modules Oct 15, 2018
@RubenVerborgh RubenVerborgh merged commit 25c2364 into nodeSolidServer:develop Oct 15, 2018
@RubenVerborgh
Copy link
Contributor

Regarding Snyk, I'm following up with @joshdcollins.

@he8us he8us deleted the feature/front-end-dependencies branch October 15, 2018 09:53
@RubenVerborgh
Copy link
Contributor

I joined Snyk and can now see the problem:

Regular Expression Denial of Service (ReDoS)

    Vulnerable module: node-forge
    Introduced through: webid@0.3.10

Detailed paths and remediation

    Introduced through: solid-server@4.1.7 › webid@0.3.10 › node-forge@0.6.49
    Remediation: No remediation path available. 

Overview

node-forge is a native implementation of TLS (and various other cryptographic tools) in JavaScript.

Affected versions of this package are vulnerable to Regular Expression Denial of Service (ReDoS) attacks. This can cause an impact of about 10 seconds matching time for data 3K characters long.

@RubenVerborgh
Copy link
Contributor

Fixed by d81182f.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants