Security Policy
TimWolla opened this issue · 10 comments
Hi! I've just updated sebastian/diff from 4.x to 5.x in our application, came across the new SECURITY.md and have some questions about that:
The security policy states that the library is intended to be used in development environments only and should not be placed onto a web server. We're developing a web application where we've just replaced our home-grown diff implementation by this library within the current in-development release – thus directly going against the new security policy. We're using the library to calculate the differences between two versions of user-generated content, similar to the version history in Wikipedia.
Are there specific/concrete concerns with using the library on a web server and passing possibly untrusted user-generated data to it? Or does the policy exist because you don't want to concern yourself with possible attack vectors during development to keep things simple for you and you don't want someone shouting at you if it breaks (but you would accept well-written reports + fixes in case something pops up)?
For your reference: WoltLab/WCF#4918, WoltLab/WCF#5285
Are there specific/concrete concerns with using the library on a web server
No.
Or does the policy exist because you don't want to concern yourself with possible attack vectors
Yes.
Background: this library was extracted out of PHPUnit, which must only be used in development environments. I recently "had to" provide security policies for libraries such as this one and the simplest thing was to base the security policy on PHPUnit's.
I should probably come up with a security policy for the libraries that were extracted from PHPUnit that is more appropriate. So thank you for bringing this up!
Perfect, thank you for your super fast response, for clarifying and of course for writing this library. We'll then continue to include it with our application.
I'll leave this issue open as a tracking issue for you with regard to a rewording of the security policy. Feel free to close whenever it suits you.
Are there specific/concrete concerns with using the library on a web server
No.
I'd disagree here ;) - but maybe that's just my level of paranoia.
The problem with this particular library is that it has to deal with unknown types of data that likely are even user generated. It might be quite feasible, even though I currently wouldn't know how exactly, to trick this library into rendering said user supplied content in a way that it would lead to XSS when run within a public website.
That's of course hardly an issue in a CLI context; but as this library has not been developed with such a use in mind, it might contain code that could be exploited given the right "input". It hasn't been audited for this kind of use scenario either, thus the warning.
It might be quite feasible, even though I currently wouldn't know how exactly, to trick this library into rendering said user supplied content in a way that it would lead to XSS when run within a public website.
This is generally something one would solve with a proper template engine that performs automated escaping of data in an HTML context. This diffing algorithm does not need to concern itself with where the data is emitted and the library does not include a HTML output builder (which would need to concern itself with escaping) by default.
The primary concern when exposed to untrusted data is likely resource exhaustion (either CPU or memory), but that really is unavoidable given larger inputs.
Totally agree. All I was trying to say is that we didn't give any non local, cli context much of a thought and people might assume the diff output is "webready" - which it certainly is not :)
Security Policy
If you believe you have found a security vulnerability in the library that is developed in this repository, please report it to us through coordinated disclosure.
Please do not report security vulnerabilities through public GitHub issues, discussions, or pull requests.
Instead, please send an email to sebastian@phpunit.de.
Please include as much of the information listed below as you can to help us better understand and resolve the issue:
- The type of issue
- Full paths of source file(s) related to the manifestation of the issue
- The location of the affected source code (tag/branch/commit or direct URL)
- Any special configuration required to reproduce the issue
- Step-by-step instructions to reproduce the issue
- Proof-of-concept or exploit code (if possible)
- Impact of the issue, including how an attacker might exploit the issue
This information will help us triage your report more quickly.
Web Context
The library that is developed in this repository
- was either extracted from PHPUnit or developed specifically as a dependency for PHPUnit
- is developed to be used in development environments and on the command-line
- does not concern itself with the web context
- neither filters input nor does it escape output
- might contain code that can be exploited given the right input
Would the above be an improvement?
The first part sounds good to me. The “Web Context” part however does not feel actionable to me as a reader. Let me suggest something along the lines of the following:
The library that is developed in this repository was either extracted from PHPUnit or developed specifically as a dependency for PHPUnit.
The library is developed with a focus on development environments and the command line. No specific testing or hardening with regard to using the library in a HTTP or web context or with untrusted input data is performed. The library might also contain functionality that intentionally exposes internal application data for debugging purposes.
If the library is used in a web application, the application developer is responsible for filtering inputs or escaping outputs as necessary and for verifying that the used functionality is safe for use within the intended context.
Vulnerabilities specific to the use outside of a development context will be fixed as applicable, provided that the fix does not have an averse effect on the primary use case for development purposes.
First paragraph sets the broader context: Primary use case is PHPUnit. Second paragraph explains possible issues when used outside the primary use case. Third paragraph moves responsibility to the user: They have to check given their use case (using sebastian/diff is likely safe for most cases, using sebastian/exporter is likely unsafe for most cases). Fourth paragraph assures them that any genuine vulnerability will still be addressed if the fix does not break the primary use case.
Thank you for your feedback!
Thank you!