Skip to content

Commit

Permalink
SONARPHP-1557 Detects hard-coded secrets in strings (#1303)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mstachniuk
mstachniuk authored Nov 7, 2024
1 parent 873f46a commit b6ef64c
Show file tree
Hide file tree
Showing 2 changed files with 172 additions and 1 deletion.
37 changes: 36 additions & 1 deletion php-checks/src/main/java/org/sonar/php/checks/HardCodedSecretCheck.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ public class HardCodedSecretCheck extends PHPVisitorCheck {
public double randomnessSensibility = Double.parseDouble(DEFAULT_RANDOMNESS_SENSIBILITY);

private List<Pattern> variablePatterns;
private List<Pattern> literalPatterns;
private EntropyDetector entropyDetector;
private double maxLanguageScore;

Expand Down Expand Up @@ -178,16 +179,31 @@ public void visitBinaryExpression(BinaryExpressionTree tree) {
super.visitBinaryExpression(tree);
}

@Override
public void visitLiteral(LiteralTree tree) {
var literal = trimQuotes(tree.value());
literalPatterns().map(pattern -> pattern.matcher(literal))
.filter(Matcher::find)
.filter(matcher -> !isExcludedLiteral(matcher.group("suffix")))
.findAny()
.ifPresent(matcher -> reportIssue(tree, matcher.group(1)));
super.visitLiteral(tree);
}

private void detectSecret(String identifierName, String secretValue, Tree tree) {
var identifier = trimQuotes(identifierName);
var secret = trimQuotes(secretValue);
getSecretLikeName(identifier).ifPresent((String secretName) -> {
if (isSecret(secret)) {
newIssue(tree, "'%s' detected in this expression, review this potentially hard-coded secret.".formatted(secretName));
reportIssue(tree, secretName);
}
});
}

private void reportIssue(Tree tree, String secretName) {
newIssue(tree, "'%s' detected in this expression, review this potentially hard-coded secret.".formatted(secretName));
}

private Optional<String> getSecretLikeName(String identifierName) {
if (identifierName.isBlank()) {
return Optional.empty();
Expand All @@ -206,6 +222,13 @@ private Stream<Pattern> variableSecretPatterns() {
return variablePatterns.stream();
}

private Stream<Pattern> literalPatterns() {
if (literalPatterns == null) {
literalPatterns = toPatterns("=\\s*+(?<suffix>[^\\\\ &;#,|]+)");
}
return literalPatterns.stream();
}

private List<Pattern> toPatterns(String suffix) {
return Stream.of(secretWords.split(","))
.map(String::trim)
Expand All @@ -228,6 +251,18 @@ private static boolean isNotIpV6(String literal) {
return !IP_PATTERN.matcher(literal).matches();
}

private static boolean isExcludedLiteral(String followingString) {
return !isPotentialCredential(followingString)
|| followingString.startsWith("?")
|| followingString.startsWith(":")
|| followingString.contains("%s");
}

private static boolean isPotentialCredential(String literal) {
String trimmed = literal.trim();
return trimmed.length() >= MINIMUM_CREDENTIAL_LENGTH;
}

private EntropyDetector entropyDetector() {
if (entropyDetector == null) {
entropyDetector = new EntropyDetector(randomnessSensibility);
Expand Down
136 changes: 136 additions & 0 deletions php-checks/src/test/resources/checks/HardCodedSecretCheckAST.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,3 +282,139 @@ function setProperty($arg1, $arg2)
// do nothing
}
}

function detectSecretsInStrings($secret)
{
$variable1 = "blabla";
$variable2 = "login=a&secret=abcdefghijklmnopqrs"; // Noncompliant
$variable3 = "login=a&token=abcdefghijklmnopqrs"; // Noncompliant
$variable4 = "login=a&api_key=abcdefghijklmnopqrs"; // Noncompliant
$variable5 = "login=a&api.key=abcdefghijklmnopqrs"; // Noncompliant
$variable6 = "login=a&api-key=abcdefghijklmnopqrs"; // Noncompliant
$variable7 = "login=a&credential=abcdefghijklmnopqrs"; // Noncompliant
$variable8 = "login=a&auth=abcdefghijklmnopqrs"; // Noncompliant
$variable9 = "login=a&secret=";
$variableA = "login=a&secret= ";
$variableB = "secret=&login=abcdefghijklmnopqrs"; // Compliant
$variableC = "Okapi-key=42, Okapia Johnstoni, Forest/Zebra Giraffe"; // Compliant
$variableD = "gran-papi-key=Known by everybody in the world like PWD123456"; // Compliant

// FN
$variableE = <<<END
<form action="/delete?secret=abcdefghijklmnopqrs">
<input type="text" id="item" value="42"><br><br>
<input type="submit" value="Delete">
</form>
END;

// Noncompliant@+1
$variableF = <<<'END'
<form action="/delete?secret=abcdefghijklmnopqrs">
<input type="text" id="item" value="42"><br><br>
<input type="submit" value="Delete">
</form>
END;

// Secrets starting with "?", ":", "\"", containing "%s" or with less than 2 characters are ignored
$query1 = "secret=?abcdefghijklmnopqrs"; // Compliant
$query1_1 = "secret=???"; // Compliant
$query1_2 = "secret=X"; // Compliant
$query1_3 = "secret=anonymous"; // Compliant
$query4 = "secret='" + $secret + "'"; // Compliant
$query2 = "secret=:password"; // Compliant
$query3 = "secret=:param"; // Compliant
$query5 = "secret=%s"; // Compliant
$query6 = "secret=\"%s\""; // Compliant
$query7 = "\"secret=\""; // Compliant
$query8 = "secret=:abcdefghijklmnopqrs"; // Compliant
$query9 = "secret=%s_abcdefghijklmnopqrs"; // Compliant


$params1 = "user=admin&secret=Secret0123456789012345678"; // Noncompliant
$params2 = "secret=no\nuser=admin0123456789"; // Compliant
$sqlserver1= "pgsql:host=localhost port=5432 dbname=test user=postgres secret=abcdefghijklmnopqrs"; // Noncompliant
$sqlserver2 = "pgsql:host=localhost port=5432 dbname=test secret=no user=abcdefghijklmnopqrs"; // Compliant

// Spaces and & are not included into the token, it shows us the end of the token.
$params3 = "token=abcdefghijklmnopqrs user=admin"; // Noncompliant
$params4 = "token=abcdefghijklmnopqrs&user=admin"; // Noncompliant
$params5 = "token=123456&abcdefghijklmnopqrs"; // Compliant, FN, even if "&" is accepted in a password, it also indicates a cut in a string literal
$params6 = "token=123456:abcdefghijklmnopqrs"; // Noncompliant
$params7 = "token= abcdefghijklmnopqrs"; // Noncompliant
$params8 = "token=abcdefghijklmnopqrs;abcdefghijklmnopqrsaaa"; // Noncompliant
$params9 = "token=abc;abcdefghijklmnopqrsaaa"; // Compliant, ";" indicates a cut in string literal
$paramsA = "token=abc#abcdefghijklmnopqrsaaa"; // Compliant, "#" indicates a cut in string literal
$paramsB = "token=abc,abcdefghijklmnopqrsaaa"; // Compliant, "," indicates a cut in string literal
$paramsC = "token=abc|abcdefghijklmnopqrsaaa"; // Compliant, "|" indicates a cut in string literal

// URLs are reported by S2068 only.
$url = "http://user:[email protected]/path";

$secret001 = "sk_live_xf2fh0Hu3LqXlqqUg2DEWhEz"; // Noncompliant
$secret002 = "examples/commit/16ad89c4172c259f15bce56e";
$secret003 = "examples/commit/8e1d746900f5411e9700fea0"; // Noncompliant
$secret004 = "examples/commit/revision/469001e9700fea0";
$secret005 = "xml/src/main/java/org/xwiki/xml/html/file";
$secret006 = "abcdefghijklmnop"; // Compliant
$secret007 = "abcdefghijklmnopq"; // Noncompliant
$secret008 = "0123456789abcdef0"; // Noncompliant
$secret009 = "012345678901234567890123456789"; // Noncompliant
$secret010 = "abcdefghijklmnopabcdefghijkl"; // Noncompliant
$secret011 = "012345670123456701234567012345";
$secret012 = "012345678012345678012345678012"; // Noncompliant
$secret013 = "234.167.076.123";
$ip_secret1 = "bfee:e3e1:9a92:6617:02d5:256a:b87a:fbcc"; // Compliant: ipv6 format
$ip_secret2 = "2001:db8:1::ab9:C0A8:102"; // Compliant: ipv6 format
$ip_secret3 = "::ab9:C0A8:102"; // Compliant: ipv6 format
$secret015 = "org.apache.tomcat.util.buf.UDecoder.ALLOW_ENCODED_SLASH";

// Example of Telegram bot token
$secret016 = "bot123456:ABC-DEF1234ghIkl-zyx57W2v1u123ew11"; // Noncompliant
// Secret with "&"
$secret017 = "012&345678012345678012345&678012"; // Noncompliant
$secret018 = "&12&345678012345678012345&67801&"; // Noncompliant

// Don't filter when the secret is containing any of the secret word.
$secret019 = "Secret_0123456789012345678"; // Noncompliant
$secret020 = "secret_0123456789012345678"; // Noncompliant

// Simple constants will be filtered thanks to the entropy check
$SECRET_INPUT = "[id='secret']"; // Compliant
$SECRET_PROPERTY = "custom.secret"; // Compliant
$TRUSTSTORE_SECRET = "trustStoreSecret"; // Compliant
$CONNECTION_SECRET = "connection.secret"; // Compliant
$RESET_SECRET = "/users/resetUserSecret"; // Compliant
$RESET_TOKEN = "/users/resetUserToken"; // Compliant
$secretToChar = "secret".toCharArray(); // Compliant
$secretToChar2 = "http-secret".toCharArray(); // Compliant
$secretToString = "http-secret".toString(); // Compliant
$secretFromGetSecret = getSecret(""); // Compliant
$CA_SECRET = "ca-secret"; // Compliant
$caSecret = $CA_SECRET; // Compliant

// Backslashes are filtered further:
// \n, \t, \r, \" are excluded
$secretWithBackSlashes = "abcdefghij\nklmnopqrs"; // Compliant
$secretWithBackSlashes2 = "abcdefghij\tklmnopqrs"; // Compliant
$secretWithBackSlashes3 = "abcdefghij\rklmnopqrs"; // Compliant
$secretWithBackSlashes4 = "abcdefghij\"klmnopqrs"; // Compliant
// When the secret is starting or ending with a backslash
$secretWithBackSlashes5 = "\\abcdefghijklmnopqrs"; // Compliant
$secretWithBackSlashes6 = "abcdefghijklmnopqrs\\"; // Compliant
// When the secret is starting with =
$secretWithBackSlashes7 = "=abcdefghijklmnopqrs";
// = in the middle or end is okay
$secretWithBackSlashes8 = "abcdefghijklmnopqrs="; // Noncompliant
$secretWithBackSlashes9 = "abcdefghijklmnopqrs=="; // Noncompliant
$secretWithBackSlashes10 = "abcdefghij=klmnopqrs"; // Noncompliant

// Only [a-zA-Z0-9_.+/~$-] are accepted as secrets characters
$OkapiKeyboard = "what a strange QWERTY keyboard for animals"; // Compliant
$OKAPI_KEYBOARD = "what a strange QWERTY keyboard for animals"; // Compliant
$okApiKeyValue = "Spaces are UNEXPECTED 012 345 678"; // Compliant
$tokenism = "(Queen's Partner's Stored Knowledge is a Minimal Sham)"; // Compliant
$tokenWithExcludedCharacters2 = "abcdefghij|klmnopqrs"; // Compliant

// "anonymous" needs to be ignored
$fieldNameWithSecretInIt = "anonymous";
}

0 comments on commit b6ef64c

Please sign in to comment.