High severity vulnerabilities

[alexkuc]

Cloning repository via npm mentions the following:

found 2 high severity vulnerabilities

Running npm audit reveals the following:

                       === npm audit security report ===

┌──────────────────────────────────────────────────────────────────────────────┐
│                                Manual Review                                 │
│            Some vulnerabilities require your attention to resolve            │
│                                                                              │
│         Visit https://go.npm.me/audit-guide for additional guidance          │
└──────────────────────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ High          │ Server-Side Request Forgery                                  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ axios                                                        │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=0.21.1                                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ browser-sync [dev]                                           │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ browser-sync > localtunnel > axios                           │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/1594                            │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ High          │ Server-Side Request Forgery                                  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ axios                                                        │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=0.21.1                                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ bundlewatch [dev]                                            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ bundlewatch > axios                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/1594                            │
└───────────────┴──────────────────────────────────────────────────────────────┘
found 2 high severity vulnerabilities in 1425 scanned packages
  2 vulnerabilities require manual review. See the full report for details.

I know that browsersync is used only for development and is not part of dist files. What about bundlewatch? Is it used inside dist files or not?

Reading description of bundlewatch leads me to believe it is used as a dev tool and as such, is not part of the dist files but I'd like to double check to be sure.

[danny007in]

Probably it might be issue with dependabot,
Need manual bump of npm dependency

[REJack]

There is no problem with dependabot, but the bundlewatch package has no new update to fix this. There is literally no fix this right now.

[alexkuc]

I'll leave these links here as they directly address the affected dependencies:

• High severity vulnerability (axios) BrowserSync/browser-sync#1831 (axios vulnerability / part of localtunnel which is used by browser-sync)
• High severity vulnerability (axios) bundlewatch/bundlewatch#313 (axios vulnerability / part of bundlewatch)

[danny007in]

Fixed in 51f243e

[REJack]

No only one of the two is fixed.

[REJack]

@XhmikosR Should we wait for bundlewatch updates the package or can I release AdminLTE v3.1.0?

[XhmikosR]

Bundlewatch is a devDependency so I'd say it shouldn't block you. The rest of the plugins issues is what I"m personally worried about...

[XhmikosR]

Ah, wait, this issue is about the devDependency vulnerabilities. I wouldn't wait because there's no sign this could be fixed soon. Let alone I doubt this issue affects AdminLTE.

At some point, if https://github.com/jackyef/bundlewatch-gh-action matures, I personally plan to move to this. Right now, it has some issues, although it works. I have a WIP upstream branch, but I'm waiting for a new version.

[danny007in]

@alexkuc its common in every node projects,
also it never ending story,
i mean this kind of issues.
Because after fixing this issue,
again new issue like this will come,
check https://stackoverflow.com/questions/50243901/found-4-vulnerabilities-on-npm-install

[alexkuc]

@danny007in @REJack

I am not sure if closing this issue is the right way to go. After all, bugs happen in software all the time and it's not a reason not to fix them.

Also, it is somewhat unappealing when you install a 3rd party dependency, get a massive red warning "Security vulnerability found", open the issue tracker for the software and find the issue closed because "this happens all the time".

It's my opinion, I could be wrong but other popular JS packages generally do not close security-related issues because they happen all the time.

Also, in the SO link you gave, the accepted answer shows a GH issue (karma-runner/karma#2994) where the vulnerability was in fact fixed.

[XhmikosR]

This a pretty serious issue. Many of the plugins have security issues and this is unrelated to the npm vulnerabilities.

[REJack]

Now this is fixed 😄 bundlewatch is updated 0.3.1.

[REJack]

@XhmikosR I'm thinking about to remove the plugins folder and use CDN's with AdminLTE v4.0.0, this sould help 🤣.

[XhmikosR]

Totally unrelated, though. The plugins do have potential security issues. The idea isn't to hide this fact, but instead report them upstream and get them fixed.

[REJack]

Ok I will remove/replace about ~80% of the plugins with the change to remove jQuery from AdminLTE, then we need to check it again.