Skip to content

Commit

Permalink
SONARPHP-957 Fix uri detection in HardCodedCredentialsCheck (#531)
Browse files Browse the repository at this point in the history
  • Loading branch information
nils-werner-sonarsource authored May 11, 2020
1 parent d7c155f commit dd9dcf1
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
*/
package org.sonar.php.checks;

import java.net.URI;
import java.net.URISyntaxException;
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
Expand All @@ -29,6 +31,7 @@
import javax.annotation.Nullable;
import org.sonar.check.Rule;
import org.sonar.check.RuleProperty;
import org.sonar.php.checks.utils.CheckUtils;
import org.sonar.php.tree.impl.PHPTree;
import org.sonar.plugins.php.api.tree.Tree;
import org.sonar.plugins.php.api.tree.Tree.Kind;
Expand All @@ -50,7 +53,6 @@ public class HardCodedCredentialsCheck extends PHPVisitorCheck {
private static final String DEFAULT_CREDENTIAL_WORDS = "password,passwd,pwd";

private static final String LITERAL_PATTERN_SUFFIX = "=(?!([\\?:']|%s))..";
private static final Pattern URI_PATTERN = Pattern.compile("\\w+://(?!user(name)?:password)(\\S+):(\\S+)@");

private static final int LITERAL_PATTERN_SUFFIX_LENGTH = LITERAL_PATTERN_SUFFIX.length();
private static final Map<String, Integer> CONNECT_FUNCTIONS = initializeConnectFunctionsMap();
Expand Down Expand Up @@ -144,9 +146,21 @@ private void checkForCredentialQuery(LiteralTree literal) {
}

private void checkForCredentialUri(LiteralTree literal) {
Matcher m = URI_PATTERN.matcher(literal.value());
if (m.find() && !m.group(2).equals(m.group(3))) {
context().newIssue(this, literal, MESSAGE_URI);
String possibleUrl = CheckUtils.trimQuotes(literal.value());
URI uri = null;

try {
uri = new URI(possibleUrl);
} catch (URISyntaxException e) {
return;
}

if (uri.getUserInfo() != null) {
String userInfo = uri.getUserInfo();
Matcher m = Pattern.compile("(\\S+):(\\S+)").matcher(userInfo);
if (m.find() && !m.group(1).equals(m.group(2)) && !userInfo.equals("user:password") && !userInfo.equals("username:password")) {
context().newIssue(this, literal, MESSAGE_URI);
}
}
}

Expand Down
16 changes: 10 additions & 6 deletions php-checks/src/test/resources/checks/HardCodedCredentialsCheck.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,19 @@ function foo() {
// The literal string doesn't contain the wordlist item matched on the variable name
const DEFAULT_AMQP_PASSWORD = 'pwd'; // Noncompliant

$uri = "scheme://user:[email protected]"; // Noncompliant
$uri = "ftp://user:[email protected]"; // Noncompliant
$uri = "ssh://user:[email protected]"; // Noncompliant
$uri = "scheme://user:@domain.com"; // Compliant
$uri = "scheme://[email protected]:80"; // Compliant
$uri = "scheme://[email protected]"; // Compliant
$uri = "scheme://domain.com/user:azerty123"; // Compliant - valid url without credentials
$uri = "scheme://admin:[email protected]"; // Compliant
$uri = "https://user:[email protected]"; // Noncompliant
$uri = "http://user:[email protected]"; // Noncompliant
$uri = "https://user:@domain.com"; // Compliant
$uri = "https://[email protected]:80"; // Compliant
$uri = "http://[email protected]"; // Compliant
$uri = "https://domain.com/user:azerty123"; // Compliant - valid url without credentials
$uri = "https://admin:[email protected]"; // Compliant
Request::create('http://user:[email protected]'); // Compliant - often used for tests
Request::create('https://username:[email protected]'); // Compliant - often used for tests
preg_match('#(^git://|\.git/?$|git(?:olite)?@|//git\.|//github.com/)#i', $url); // Compliant
$gitUri = "https://user:[email protected]/username/repository.git"; // Noncompliant

ldap_bind("a", "b", "p4ssw0rd"); // Noncompliant
$conection = new PDO("a", "b", "p4ssw0rd"); // Noncompliant
Expand Down

0 comments on commit dd9dcf1

Please sign in to comment.