[BUG] potential security issues in plugins

[XhmikosR]

You could add lgtm.com to the repo: https://lgtm.com/projects/g/ColorlibHQ/AdminLTE?mode=tree after making a config to mark the files in plugins/ as 3rd-party. That being said, there are a few plugins with potential security issues: https://lgtm.com/projects/g/ColorlibHQ/AdminLTE?mode=tree&severity=&ruleFocus=1511421786841

I'd start by checking if there are any new versions of those plugins. Then, I'd try bugging the plugin authors to have a look. If those plugins are no longer maintained IMHO you shouldn't include them.

BTW GitHub has announced the successor of LGTM on GitHub Universe: https://github.blog/changelog/2020-05-06-github-advanced-security-code-scanning-now-available-in-limited-public-beta/

I'd also register for the beta and enable CodeQL when approved.

PS. You should fix https://lgtm.com/projects/g/ColorlibHQ/AdminLTE?mode=tree&severity=&ruleFocus=1815071526 and https://lgtm.com/projects/g/ColorlibHQ/AdminLTE?mode=tree&severity=&ruleFocus=1813441247 since those are in your HTML code :) You would probably find those if you added vnu-jar in your pipeline for HTML validation.

[XhmikosR]

Forgot to mention, you should also enable automated security fixes.

[REJack]

I'll look over it tomorrow 😄 and fix the bugs in my HTML files and may I add lgtm.com to the repo, but I need to check if I can do it directly or one of the group admins need to do this.

[XhmikosR]

You should be able to do lgtm and automated security fixes if you have admin rights in the repo.

[REJack]

I've added lgtm to this repo, may you can create a lgtm.yml for it 😄 Idk why it shows python and where to deactivate it 🤦‍♂️.

[XhmikosR]

Did you also enable automated security fixes for the repo?

I'm not sure how to configure lgtm further, I never had it show Python... They have support if you want to tackle this later, but I'm not sure it's reasonable to include all plugins in the scan, because it takes like ~18 minutes to finish.

[REJack]

Did you also enable automated security fixes for the repo?

Yep I enabled it 😄

[XhmikosR]

I guess we could make a PR with the LGTM plugins exclusion reverted to see what issues are left? It won't be for merging, just to see what alerts we'll get.

[REJack]

That's a nice idea :) feel free to create a PR for it.

[XhmikosR]

I can't filter the results so it's sort of unusable :/ https://lgtm.com/projects/g/ColorlibHQ/AdminLTE/rev/pr-afc34eab15c62bc90543a89c7f8c7388d8f1e54f

[REJack]

I thought the same :/

[XhmikosR]

I managed to set up CodeQL including the plugins temporarily (it takes a long time) and you can see the results here: https://github.com/XhmikosR/AdminLTE/security/code-scanning?query=ref%3Arefs%2Fheads%2Fcodeql

I haven't reviewed the warnings but:

• jqvmap seems outdated and insecure
• flot-old has an XSS which seems to be fixed in the latest flot version, so IMHO flot-old should be removed ASAP
• the rest should be probably reported upstream and fixed there; if some weeks pass and they are not tackled, I'd start looking for alternative plugins and removing the insecure ones

Now, you can register for CodeQL and we should add it, but scanning all plugins takes like 15min, so I'm not sure if it makes sense to scan the plugins.

[XhmikosR]

BTW this issue isn't fixed, it shouldn't be marked as done.

[XhmikosR]

This should be higher priority. LGTM already warns you on PRs but it seems the warnings are ignored. You shouldn't ignore the warnings.

I'll make a PR to add CodeQL when I have some time too.

[REJack]

I was thinking about to remove the plugins from repo and create a new one only for the plugins with v4. I have glue how to fix these warnings properly 😓.

[XhmikosR]

You need to use find, text when possible and in cases you construct HTML through user sources, that HTML needs to be sanitized. We have a built-in sanitizer upstream we use in popover and tooltip ourselves, which covers some XSS cases.

[REJack]

I've fixed the alerts from LGTM, you can check it 😄

[XhmikosR]

https://github.com/ColorlibHQ/AdminLTE/security/code-scanning?page=1&query=ref%3Arefs%2Fheads%2Fcodeql-test

I've made a PR #3309, but I have excluded the plugins folders. IMHO you should report these issues on their repos upstream and/or try to find alternative plugins for any old, insecure plugins.

I might change #3309 to scan the plugins folders so that any issues are visible.

EDIT: allowing to scan the plugins increases the time a lot, so I'll leave it excluded for now. Check our my comment on #3309.

[craph]

any updates about this bug ?