search

thedevdojo / voyager

Voyager - The Missing Laravel Admin
https://voyager.devdojo.com
MIT License
11.77k stars 2.67k forks source link

Security: Voyager Needs a Security Issue Tracker and Response Policy #3438

Closed dereks closed 6 years ago

dereks commented 6 years ago

Version information

Description

The security patch for bug #3267 has been sitting idle with "needs review" for almost two months. That kind of practice is not acceptable to my business clients.

The Voyager team needs a formal procedure for tracking security issues and responding to them in a timely manner. The response doesn't need to be a hotfix -- but it does need to be a response that the issue has been assessed, verified, assigned a severity level, and a list of the Voyager version numbers affected.

Consider that an administrator needs to be able to sign off on a bunch of old Voyager installs by checking the Voyager version numbers for known security issues. Every security issue need to be tagged with a tracking number, whether it be a CVE or something else. One quick and easy option is to just use a "Security" label in the GitHub issues, and then in the documentation tell people how to search for just those "Security" issues on GitHub.

For example:

https://security.calpoly.edu/content/standards/web-app-vulnerabilities

This is common best practices.

Additional context

Note that Laravel bugs are already being tracked in CVE.

https://www.cvedetails.com/vulnerability-list/vendor_id-16542/Laravel.html

It might be worth working with the core Laravel team to do something like this for all Laravel plugins. WordPress did it (jus' sayin'):

https://wpvulndb.com/

Finally, related to having team security policy is a documented security audit of your app. OWASP gives a good checklist for an audit. (Most of the OWASP stuff is automatically handled by Laravel, like SQL injection, CSRF, etc., but Voyager should be separately audited.)

See also Bug #3267 and its PR.

fletch3555 commented 6 years ago

We're a volunteer group maintaining a package that is primarily intended for dev purposes. Security issues are primarily an implementation concern.

If there is a security issue that is brought up here, we will respond to it. But we don't need a separate issue tracker for that.

dereks commented 6 years ago

a package that is primarily intended for dev purposes.

Oh. That is not what was advertised. The intro "What is Voyager" says it's "an admin interface" for "administrative tasks". Administrators care deeply about security and vulnerability vetting.

https://voyager.readme.io/docs/what-is-voyager

But we don't need a separate issue tracker for that.

Above I recommended just using this GitHub Issue Tracker.

If the project would

then it would have the accounting necessary for an administrator to vet a production system.

fletch3555 commented 6 years ago

I guess I should clarify that, since that's not entirely how I meant it. Voyager is an admin tool, however much of it's built-in functionality is an inherent security risk, and therefore intended for dev purposes only, like the database and BREAD editor. You really shouldn't be changing those in prod, and instead should be doing so in dev and migrating them to prod.

The big thing is that as a package, we have chosen not to force any particular workflow on our users. If you choose to use the database editor in prod, you can, but you also bear the responsibility to make sure it remains secure.

As for tracking security issues, I'm not opposed to adding a new tag for those. We don't have one at this point due to the infrequency of their occurrences. As I've stated a number of times already, I don't see too many things as TRUE security issues with this package. Yes, #3032 is definitely one, but I'm not aware of any others that would note any level of severity.

MrCrayon commented 6 years ago

@fletch3555

Yes, #3032 is definitely one, but I'm not aware of any others that would note any level of severity.

A bigger security issues IMHO is that a user can change is own role without permission of editing users or editing roles, that's going on since more than a year AFAIK #911

I pointed this out several times both here and in Slack group and I made a PR #3016.

I like this package, I'm using it and I'd like to contribute but I don't understand much how are decisions taken about what to merge or not, or how security issues are taken care of.

github-actions[bot] commented 4 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. If you have further questions please ask in our Slack group.