kalessil / phpinspectionsea

A Static Code Analyzer for PHP (a PhpStorm/Idea Plugin)
https://plugins.jetbrains.com/plugin/7622?pr=phpStorm
Other
1.44k stars 119 forks source link

Detecting XSS in PHP views #1397

Closed SpyroTEQ closed 4 years ago

SpyroTEQ commented 5 years ago

One of the main security issue in PHP could be XSS vulnerabilities. THis easily occur in newbies projects, with plain simple reflected stuff like:

echo "Hello, {$_GET['username']}";

But this can also occur in legacy projects, with more complex stored XSS or so, like

$value = $pdo->query('SELECT name FROM user WHERE id = ?', array($_GET['id']))->fetchAll()[0] ?? 'Anonymous';
echo "Hello, <i>" . $value . "</i>";

Some automated tools, like Coverity, might miss such things, because the DB source is unknown, or the DB source seems not injectable right now, or whatever. Plus, it's security tools is often either expensive, or uber complex to setup.

Sonar would have been a neat way to check for that, but it does not handle multiple languages, so stuff like:

echo "<i>" . $unsafeData . "</i>";

Will be hard to distinguish against stuff like

header('Content-Type: text/plain');
echo $unsafeButNotXssSensitive;

IntelliJ would be a great tool for that, as we already have all language parsing embeded. And if PHP EA boils up security issue when developer is typing code, then we can:

So, I would like to contribute to PHP EA by starting setting up XSS inspections. I'm actually a security product analist and pentester, so that would even help me find security issues in whitebox tested projects. Hence, it will serve me one way another, so why not trying to integrate this with PHP EA so it serves others?!

If so, I think I'll need to:

so we avoid false positives

Would that be a neat feature to work on for contributing to PHP EA? Or maybe I should start with potentially "simpler", like SQL injection detection (but I encounter less of them, so... both suits me)?

kalessil commented 5 years ago

I'd try collaborating on XSS for the Extended-version (there is XSS detection in Ultimate-plugin, but it's for completely different scope):

Do you want to do actual coding, or focus more on requirements engineering?

SpyroTEQ commented 5 years ago

I'm not sure what you mean by "requirements engineering", so there would be my proposal:

I do have actual PHP projects and so one way another, I will need to detect XSS in there. So I could fork the PHP EA project, integrate the use case I need in there, and from there, we might see what is generalisable to PHP EA and how we could do so? In case of failure, I'll either keep the fork to still have these inspections, or extract them into a dedicated plugin for my single case (= put it into my current personnal plugin).

I feel more confortable working with an actual specific context, even if small or maybe very scoped, rather than trying to thing globally at first glance. Besides, do you have a read me to contribute to PHP EA? It seems rather more complex than just opening an IntelliJ plugin and right click "prepare for deployment" :/ I didn't see any in the github codebase

kalessil commented 5 years ago

@SpyroTEQ: forking - generalizing will work for me =)

If you used Idea as IDE, you can import the project (it's Graddle project type). If you have a Java SDK (the one from Oracle) already, then Graddle will take care of build/test routines.

Like here: image When it comes to the IDE/EA APIs, this inspection - src/main/java/com/kalessil/phpStorm/phpInspectionsEA/inspectors/security/NonSecureCryptUsageInspector.java will be a good starting point to look at.

SpyroTEQ commented 5 years ago

Alright, but since we are behind a giant MITM proxy here, do you have any idea how to bypass (or actually fix) the Caused by: org.gradle.internal.resource.transport.http.HttpRequestException: Could not GET 'https://jetbrains.bintray.com/intellij-third-party-dependencies/gradle/plugin/org/jetbrains/intellij/plugins/gradle-intellij-plugin/0.3.7/gradle-intellij-plugin-0.3.7.pom'... that comes from Caused by: sun.security.validator.ValidatorException: PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target that I get when importing the gradle project?!...

kalessil commented 5 years ago

I pushed changes (workaround, to try), so gradle uses http instead of https repositories, can you give it a try?

SpyroTEQ commented 5 years ago

Sorry for late reply. Ahah, that (use http instead of https in the two build.gradle repos lines)is exactly what I've tried few hours after my post : ) But the same problem occurs later on: Caused by: org.gradle.internal.resource.transport.http.HttpRequestException: Could not GET 'https://cache-redirector.jetbrains.com/www.jetbrains.com/intellij-repository/releases/com/jetbrains/intellij/idea/ideaIU/2016.2.5/ideaIU-2016.2.5.pom'.

I'll check where Gradle stores its pom and components, and I'll try to manually download the missing ones (the webbrowser is configured to trust that MITM proxy... [sic]), you may revert the http/https changes so other dev won't have security issues (using plain http for such thing than dependencies seems bad to me: fixing my MITM issue should not lead to allow MITM for other devs)

It seems I could use mavenLocal() repository (as I am more used to maven), and so I can put the dependencies mannually in there in case of failure. But I get stuck at this dependency: com.jetbrains.intelliJ;idea:ideaIU:2016.2.5 I've added it (manually) into my maven cache. It worked well for some other dependency (like classworlds, see this part of the log when running gradle -d build):

10:28:13.951 [DEBUG] [org.gradle.api.internal.artifacts.ivyservice.ivyresolve.RepositoryChainComponentMetaDataResolver] Attempting to resolve component for classworlds:classworlds:1.1-alpha-2 using repositories [MavenLocal, Gradle Central Plugin Repository]
10:28:13.951 [DEBUG] [org.gradle.api.internal.artifacts.repositories.resolver.DefaultExternalResourceArtifactResolver] Loading file:/C:/Users/212636336/.m2/repository/classworlds/classworlds/1.1-alpha-2/classworlds-1.1-alpha-2.pom
10:28:13.952 [DEBUG] [org.gradle.api.internal.artifacts.repositories.resolver.DefaultExternalResourceArtifactResolver] Loading file:/C:/Users/212636336/.m2/repository/classworlds/classworlds/1.1-alpha-2/classworlds-1.1-alpha-2.jar

But this one (ideaIU) still fails. Thing is, Gradle seems to look for it in "maven" repository, not in "MavelLocal":

10:28:14.053 [DEBUG] [org.gradle.api.internal.artifacts.ivyservice.ivyresolve.RepositoryChainComponentMetaDataResolver] Attempting to resolve component for com.jetbrains.intellij.idea:ideaIU:2016.2.5 using repositories [maven]

The difference could be from lines above this log, but I don't know how to tell Gradle to look for this ideaIU dependency in MavenLocal, and not in "maven"

10:28:14.051 [INFO] [org.jetbrains.intellij.IntelliJPlugin] Configuring IntelliJ IDEA dependency
10:28:14.051 [INFO] [org.jetbrains.intellij.IntelliJPlugin] Using IDE from remote repository
10:28:14.051 [DEBUG] [org.jetbrains.intellij.IntelliJPlugin] Adding IntelliJ IDEA repository: https://cache-redirector.jetbrains.com/www.jetbrains.com/intellij-repository/releases
10:28:14.051 [DEBUG] [org.jetbrains.intellij.IntelliJPlugin] Adding IntelliJ IDEA dependency
10:28:14.052 [DEBUG] [org.gradle.internal.operations.DefaultBuildOperationExecutor] Build operation 'Resolve dependencies of :detachedConfiguration1' started
10:28:14.052 [DEBUG] [org.gradle.api.internal.artifacts.ivyservice.resolveengine.DefaultArtifactDependencyResolver] Resolving configuration ':detachedConfiguration1'
10:28:14.052 [DEBUG] [org.gradle.api.internal.artifacts.ivyservice.modulecache.ResolvedArtifactCaches] Creating new in-memory cache for repo 'maven' [73520db826dde63c67f6b3554017db77].
10:28:14.053 [DEBUG] [org.gradle.api.internal.artifacts.ivyservice.resolveengine.graph.builder.DependencyGraphBuilder] Visiting configuration com.kalessil.phpStorm.phpInspectionsEA:phpinspectionsea:unspecified(detachedConfiguration1).

Any idea? (or other ways to avoid this original SSL issue, like replacing this https://cache-redirector with http:// [but I have no idea where this is set]?)

kalessil commented 5 years ago

Ok, I'll revert back to use https then. cache-redirector is seem to be part if bintray-repositories discovery, so no chance.

According to https://docs.gradle.org/current/userguide/repository_types.html#sub:maven_local, in order to use locally cached dependecies it'll be enough to replace 'mavenCentral()' with 'mavenLocal()' in graddle.properties

I also wondering if feeding graddle implicitly with proxy settings (in the graddle.properties) will work: https://stackoverflow.com/a/37079513.

SpyroTEQ commented 5 years ago

That's 'the gradle properties proxy) what I tried first, but the issue is that the proxy-MITM certificate is not part of Java's trusted store, and I failed to add the proper certificate to that store...

I suppose I'll need to find a manual way for bypassing this crappy proxy...

kalessil commented 4 years ago

I reverted back to https, any luck with solving proxy restrictions?

SpyroTEQ commented 4 years ago

I guess no... I'll retry someday, but you can give no consideration to this issue: keep your setup and configs all safe and clean ;)