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

Fix security issues #76

Merged
merged 5 commits into from Jun 18, 2019
Merged

Conversation

kinow
Copy link
Member

@kinow kinow commented May 23, 2019

Reported by GitHub:

image

So first did a npm audit:

                                                                                
                       === npm audit security report ===                        
                                                                                
# Run  npm update fstream --depth 4  to resolve 2 vulnerabilities
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ High          │ Arbitrary File Overwrite                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ fstream                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ node-sass [dev]                                              │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ node-sass > node-gyp > fstream                               │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/886                             │
└───────────────┴──────────────────────────────────────────────────────────────┘


┌───────────────┬──────────────────────────────────────────────────────────────┐
│ High          │ Arbitrary File Overwrite                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ fstream                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ node-sass [dev]                                              │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ node-sass > node-gyp > tar > fstream                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/886                             │
└───────────────┴──────────────────────────────────────────────────────────────┘


# Run  npm update tar --depth 3  to resolve 1 vulnerability
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ High          │ Arbitrary File Overwrite                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ tar                                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ node-sass [dev]                                              │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ node-sass > node-gyp > tar                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/803                             │
└───────────────┴──────────────────────────────────────────────────────────────┘


found 3 high severity vulnerabilities in 35410 scanned packages
  run `npm audit fix` to fix 3 of them.

Then checked remaining dependencies with vue ui and npm:

kinow@ranma:~/Development/python/workspace/cylc-ui$ npm outdated
Package                            Current         Wanted         Latest  Location
@vue/cli-plugin-babel                3.4.1          3.7.0          3.7.0  cylc-ui
@vue/cli-plugin-e2e-cypress          3.4.1          3.7.0          3.7.0  cylc-ui
@vue/cli-plugin-eslint               3.4.1          3.7.0          3.7.0  cylc-ui
@vue/cli-plugin-unit-mocha           3.4.1          3.7.0          3.7.0  cylc-ui
apollo-boost                         0.3.1          0.3.1          0.4.0  cylc-ui
graphql                             14.2.1         14.3.1         14.3.1  cylc-ui
nyc                                 14.1.0         14.1.1         14.1.1  cylc-ui
vee-validate                         2.2.6          2.2.8          2.2.8  cylc-ui
vue-apollo                   3.0.0-beta.29  3.0.0-beta.30  3.0.0-beta.30  cylc-ui
vue-cli-plugin-apollo               0.19.1         0.19.2         0.20.0  cylc-ui
vuex                                 3.1.0          3.1.1          3.1.1  cylc-ui
webpack                             4.30.0         4.32.2         4.32.2  cylc-ui

Finally running npm update to update these dependencies. Followed by checking the current webpack-bundle-analyzer:

$ npm list | grep webpack-bundle-analyzer
│ ├─┬ webpack-bundle-analyzer@3.3.2

@kinow kinow self-assigned this May 23, 2019
@kinow kinow added the bug Something isn't working label May 23, 2019
@codecov-io
Copy link

codecov-io commented May 23, 2019

Codecov Report

Merging #76 into master will increase coverage by 0.8%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master      #76     +/-   ##
=========================================
+ Coverage   77.41%   78.22%   +0.8%     
=========================================
  Files          13       13             
  Lines         124      124             
=========================================
+ Hits           96       97      +1     
+ Misses         28       27      -1
Impacted Files Coverage Δ
src/store/index.js 95.65% <0%> (+4.34%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb9538c...7a0aac7. Read the comment docs.

@kinow
Copy link
Member Author

kinow commented May 30, 2019

Hmm. GitHub found another vulnerability today in some JS library. Will check tomorrow, and if there is a fix for this new vulnerability, then I will close this one and raise a new pull request to the old and new issues.

@kinow
Copy link
Member Author

kinow commented May 30, 2019

We had a new security vulnerability, now in axios, used for HTTP requests.

image

Just did a ncu -u, npm install, and updated the axios version 0.19.0, which is not affected. Note too, that this is more dangerous for developers using axios in Node.js servers:

As mentioned on the Snyk blog

If you are requesting resources from untrusted sources, or via insecure mediums, then you are potentially vulnerable to Denial of Service where malicious users can control the remote resource.

When using axios in a Node.js server this can be disastrous due to the single threaded nature of the runtime. A spike in resources such as I/O and CPU negatively affect all users connected to that server. In browser environments, the Denial of Service negatively affects end-users with varying severity, depending on how the resource fetching with axios is used in the application.

@kinow
Copy link
Member Author

kinow commented May 30, 2019

And the latest result of ncu -u:

$ ncu -u
Upgrading /home/kinow/Development/python/workspace/cylc-ui/package.json
[====================] 41/41 100%

 apollo-boost                  ^0.3.1  →   ^0.4.0 
 chartist                      0.11.0  →   0.11.2 
 vee-validate                  ^2.2.8  →   ^2.2.9 
 vue-chartist                  ^2.1.2  →   ^2.2.0 
 @vue/cli-plugin-babel         ^3.7.0  →   ^3.8.0 
 @vue/cli-plugin-e2e-cypress   ^3.7.0  →   ^3.8.0 
 @vue/cli-plugin-eslint        ^3.7.0  →   ^3.8.0 
 @vue/cli-plugin-unit-mocha    ^3.7.0  →   ^3.8.0 
 @vue/cli-service              ^3.7.0  →   ^3.8.0 
 axios                        ^0.18.0  →  ^0.19.0 
 chai                          ^4.1.2  →   ^4.2.0 
 stylus-loader                 ^3.0.1  →   ^3.0.2 
 vue-cli-plugin-apollo        ^0.19.2  →  ^0.20.0 

Run npm install to install new versions.

@kinow
Copy link
Member Author

kinow commented May 31, 2019

Yet another security vulnerability popped up just now on GitHub:

image

But I don't think we need to worry, as this PR already covers that one too.

image

Looks like it was included in the automatic updates, and GitHub alerts probably index the repo daily. 👍

@kinow
Copy link
Member Author

kinow commented Jun 5, 2019

And yet another security vulnerability! 🎉 js-yaml this time.

image

@kinow
Copy link
Member Author

kinow commented Jun 5, 2019

Oh, and axios has one too, not found by GitHub security analyzer.

image

                      === npm audit security report ===                        
                                                                                
# Run  npm install --save-dev axios@0.19.0  to resolve 1 vulnerability
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Moderate      │ Denial of Service                                            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ axios                                                        │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ axios [dev]                                                  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ axios                                                        │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/880                             │
└───────────────┴──────────────────────────────────────────────────────────────┘


found 1 moderate severity vulnerability in 35353 scanned packages
  run `npm audit fix` to fix 1 of them.

@kinow
Copy link
Member Author

kinow commented Jun 5, 2019

More dependencies to be updated by ncu:

ncu
Checking /home/kinow/Development/python/workspace/cylc-ui/package.json
[====================] 41/41 100%

 apollo-boost                  ^0.3.1  →   ^0.4.1 
 chartist                      0.11.0  →   0.11.2 
 vee-validate                  ^2.2.8  →   ^2.2.9 
 vue-chartist                  ^2.1.2  →   ^2.2.0 
 @vue/cli-plugin-babel         ^3.7.0  →   ^3.8.0 
 @vue/cli-plugin-e2e-cypress   ^3.7.0  →   ^3.8.0 
 @vue/cli-plugin-eslint        ^3.7.0  →   ^3.8.0 
 @vue/cli-plugin-unit-mocha    ^3.7.0  →   ^3.8.0 
 @vue/cli-service              ^3.7.0  →   ^3.8.0 
 chai                          ^4.1.2  →   ^4.2.0 
 stylus-loader                 ^3.0.1  →   ^3.0.2 
 vue-cli-plugin-apollo        ^0.19.2  →  ^0.20.0 
 webpack                      ^4.32.2  →  ^4.33.0 

Run ncu -u to upgrade package.json

@kinow
Copy link
Member Author

kinow commented Jun 5, 2019

js-yaml is a transitive dependency in cylc-ui. It is a dependency of eslint, which is a dependency of @vue/cli-plugin-eslint (this last one used in our project). With the upgrade

 @vue/cli-plugin-eslint        ^3.7.0  →   ^3.8.0 

Looks like the issue was fixed:

$ npm list | grep js-yaml
│ │ ├── js-yaml@3.13.1 deduped
│ │ │ │   ├── js-yaml@3.13.1 deduped
npm ERR! peer dep missing: typescript@>=2.0, required by ts-node@8.2.0
│ │ │ ├── js-yaml@3.13.1 deduped
│ │ │ │ ├── js-yaml@3.13.1 deduped
│ ├─┬ js-yaml@3.13.1
│ ├─┬ js-yaml@3.13.1

They recommend updating to this version:

Screen Shot 2019-06-06 at 10 35 33-fullpage

Rebased PR, sending updates in new commits now. PR should be good to be reviewed again now 😬

@kinow kinow changed the title Fix security issue in webpack-bundle-analyzer Fix security issues Jun 8, 2019
@kinow
Copy link
Member Author

kinow commented Jun 8, 2019

Another one, now querystringify.

image

Upgrading /home/kinow/Development/python/workspace/cylc-ui/package.json
[====================] 41/41 100%

 apollo-boost  ^0.4.1  →  ^0.4.2 

Run npm install to install new versions.

Updated dependencies again, but checking querystringify, looks like it was fixed before, by one of the other commits in this PR already. This is a dependency of vue-cli-service, and we are using 2.1.1 (>2.0.0 as per security requirement).

$ npm list
...
│ │ │ └─┬ url-parse@1.4.7
│ │ │   ├── querystringify@2.1.1

Screen Shot 2019-06-08 at 14 21 58-fullpage

@matthewrmshin matthewrmshin merged commit 201f89d into cylc:master Jun 18, 2019
@kinow kinow added this to the 0.1 milestone Sep 10, 2019
@kinow kinow deleted the fix-security-issues-2 branch October 30, 2019 02:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants