From fb2f5b69d21d0b7870d4bb5ae8912d4132e73f6e Mon Sep 17 00:00:00 2001 From: Marcin Stachniuk Date: Tue, 29 Oct 2024 17:20:26 +0100 Subject: [PATCH 1/3] SONARPHP-1369 Rule S6418: Hard-coded secrets are security-sensitive --- .../php/checks/HardCodedSecretCheck.java | 126 ++++++++++++++++++ .../php/checks/HardCodedSecretCheckTest.java | 12 ++ .../resources/checks/HardCodedSecretCheck.php | 53 ++++++++ 3 files changed, 191 insertions(+) create mode 100644 php-checks/src/main/java/org/sonar/php/checks/HardCodedSecretCheck.java create mode 100644 php-checks/src/test/java/org/sonar/php/checks/HardCodedSecretCheckTest.java create mode 100644 php-checks/src/test/resources/checks/HardCodedSecretCheck.php diff --git a/php-checks/src/main/java/org/sonar/php/checks/HardCodedSecretCheck.java b/php-checks/src/main/java/org/sonar/php/checks/HardCodedSecretCheck.java new file mode 100644 index 000000000..9bcf0f88d --- /dev/null +++ b/php-checks/src/main/java/org/sonar/php/checks/HardCodedSecretCheck.java @@ -0,0 +1,126 @@ +package org.sonar.php.checks; + +import java.util.Iterator; +import java.util.List; +import java.util.Optional; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.stream.Stream; +import org.sonar.check.Rule; +import org.sonar.check.RuleProperty; +import org.sonar.php.checks.utils.CheckUtils; +import org.sonar.plugins.php.api.tree.Tree; +import org.sonar.plugins.php.api.tree.declaration.ConstantDeclarationTree; +import org.sonar.plugins.php.api.tree.declaration.MethodDeclarationTree; +import org.sonar.plugins.php.api.tree.declaration.VariableDeclarationTree; +import org.sonar.plugins.php.api.tree.expression.ExpandableStringCharactersTree; +import org.sonar.plugins.php.api.tree.expression.ExpressionTree; +import org.sonar.plugins.php.api.tree.expression.FunctionCallTree; +import org.sonar.plugins.php.api.tree.expression.HeredocStringLiteralTree; +import org.sonar.plugins.php.api.tree.expression.LiteralTree; +import org.sonar.plugins.php.api.visitors.PHPVisitorCheck; + +@Rule(key = "S6418") +public class HardCodedSecretCheck extends PHPVisitorCheck { + private static final String DEFAULT_SECRET_WORDS = "api[_.-]?key,auth,credential,secret,token"; + private static final String DEFAULT_RANDOMNESS_SENSIBILITY = "5.0"; + + @RuleProperty( + key = "secretWords", + description = "Comma separated list of words identifying potential secrets", + defaultValue = DEFAULT_SECRET_WORDS) + public String secretWords = DEFAULT_SECRET_WORDS; + + @RuleProperty( + key = "randomnessSensibility", + description = "Allows to tune the Randomness Sensibility (from 0 to 10)", + defaultValue = DEFAULT_RANDOMNESS_SENSIBILITY) + public double randomnessSensibility = Double.parseDouble(DEFAULT_RANDOMNESS_SENSIBILITY); + + private List variablePatterns = null; + + + @Override + public void visitConstDeclaration(ConstantDeclarationTree tree) { + //TODO remove + for (Iterator it = tree.declarations().elementsAndSeparators(); it.hasNext(); ) { + Tree elementsOrSeparator = it.next(); + if (elementsOrSeparator.is(Tree.Kind.VARIABLE_DECLARATION)) { + var variableDeclaration = (VariableDeclarationTree) elementsOrSeparator; + visitVariableDeclaration(variableDeclaration); + } + } + } + + @Override + public void visitVariableDeclaration(VariableDeclarationTree tree) { +// Pattern.compile("=\\s*+([^\\\\ &;#,|]+)"); + System.out.println("BBBB variable declaration: " + tree.identifier() + " kind " + tree.getKind()); + if (tree.initValue() instanceof LiteralTree literalTree) { + detectSecret(tree.identifier().text(), CheckUtils.trimQuotes(literalTree.value()), literalTree); + } + if (tree.initValue() instanceof HeredocStringLiteralTree heredoc) { + for (ExpandableStringCharactersTree heredocLine : heredoc.strings()) { + detectSecret(tree.identifier().text(), heredocLine.value(), heredocLine); + } + } + } + + @Override + public void visitFunctionCall(FunctionCallTree functionCall) { + if ("define".equals(CheckUtils.getLowerCaseFunctionName(functionCall))) { + CheckUtils.argumentValue(functionCall, "constant_name", 0) + .filter(constantName -> constantName.is(Tree.Kind.REGULAR_STRING_LITERAL)) + .map(LiteralTree.class::cast) + .ifPresent(constantName -> { + //TODO remove + System.out.println("DDDD constantName " + constantName.value()); + CheckUtils.argumentValue(functionCall, "value", 1) + .filter(value -> value.is(Tree.Kind.REGULAR_STRING_LITERAL)) + .map(LiteralTree.class::cast) + .ifPresent(value -> { + //TODO remove + System.out.println("DDDD value: " + value.value()); + detectSecret(CheckUtils.trimQuotes(constantName.value()), CheckUtils.trimQuotes(value.value()), value); + }); + + }); + } + super.visitFunctionCall(functionCall); + } + + private void detectSecret(String identifierName, String secret, Tree tree) { + //TODO remove +// System.out.println("AAAA detectSecret " + identifierName); + getSecretLikeName(identifierName).ifPresent(secretName -> { + if (secret.equals("abcdefghijklmnopqrs")) { + newIssue(tree, "'%s' detected in this expression, review this potentially hard-coded secret.".formatted(secretName)); + } + }); + } + + private Optional getSecretLikeName(String identifierName) { + if (identifierName.isBlank()) { + return Optional.empty(); + } + return variableSecretPatterns() + .map(pattern -> pattern.matcher(identifierName)) + .filter(Matcher::find) + .map(matcher -> matcher.group(1)) + .findAny(); + } + + private Stream variableSecretPatterns() { + if (variablePatterns == null) { + variablePatterns = toPatterns(""); + } + return variablePatterns.stream(); + } + + private List toPatterns(String suffix) { + return Stream.of(secretWords.split(",")) + .map(String::trim) + .map(word -> Pattern.compile("(" + word + ")" + suffix, Pattern.CASE_INSENSITIVE)) + .toList(); + } +} diff --git a/php-checks/src/test/java/org/sonar/php/checks/HardCodedSecretCheckTest.java b/php-checks/src/test/java/org/sonar/php/checks/HardCodedSecretCheckTest.java new file mode 100644 index 000000000..71492a6da --- /dev/null +++ b/php-checks/src/test/java/org/sonar/php/checks/HardCodedSecretCheckTest.java @@ -0,0 +1,12 @@ +package org.sonar.php.checks; + +import org.junit.jupiter.api.Test; +import org.sonar.plugins.php.CheckVerifier; + +class HardCodedSecretCheckTest { + + @Test + void test() throws Exception { + CheckVerifier.verify(new HardCodedSecretCheck(), "HardCodedSecretCheck.php"); + } +} diff --git a/php-checks/src/test/resources/checks/HardCodedSecretCheck.php b/php-checks/src/test/resources/checks/HardCodedSecretCheck.php new file mode 100644 index 000000000..fb629c588 --- /dev/null +++ b/php-checks/src/test/resources/checks/HardCodedSecretCheck.php @@ -0,0 +1,53 @@ + Date: Wed, 30 Oct 2024 10:34:54 +0100 Subject: [PATCH 2/3] Fix headers and formatting --- .../php/checks/HardCodedSecretCheck.java | 44 +++++++++++++------ .../php/checks/HardCodedSecretCheckTest.java | 19 ++++++++ 2 files changed, 49 insertions(+), 14 deletions(-) diff --git a/php-checks/src/main/java/org/sonar/php/checks/HardCodedSecretCheck.java b/php-checks/src/main/java/org/sonar/php/checks/HardCodedSecretCheck.java index 9bcf0f88d..a725361ae 100644 --- a/php-checks/src/main/java/org/sonar/php/checks/HardCodedSecretCheck.java +++ b/php-checks/src/main/java/org/sonar/php/checks/HardCodedSecretCheck.java @@ -1,3 +1,22 @@ +/* + * SonarQube PHP Plugin + * Copyright (C) 2010-2024 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ package org.sonar.php.checks; import java.util.Iterator; @@ -11,10 +30,8 @@ import org.sonar.php.checks.utils.CheckUtils; import org.sonar.plugins.php.api.tree.Tree; import org.sonar.plugins.php.api.tree.declaration.ConstantDeclarationTree; -import org.sonar.plugins.php.api.tree.declaration.MethodDeclarationTree; import org.sonar.plugins.php.api.tree.declaration.VariableDeclarationTree; import org.sonar.plugins.php.api.tree.expression.ExpandableStringCharactersTree; -import org.sonar.plugins.php.api.tree.expression.ExpressionTree; import org.sonar.plugins.php.api.tree.expression.FunctionCallTree; import org.sonar.plugins.php.api.tree.expression.HeredocStringLiteralTree; import org.sonar.plugins.php.api.tree.expression.LiteralTree; @@ -39,11 +56,10 @@ public class HardCodedSecretCheck extends PHPVisitorCheck { private List variablePatterns = null; - @Override public void visitConstDeclaration(ConstantDeclarationTree tree) { - //TODO remove - for (Iterator it = tree.declarations().elementsAndSeparators(); it.hasNext(); ) { + // TODO remove + for (Iterator it = tree.declarations().elementsAndSeparators(); it.hasNext();) { Tree elementsOrSeparator = it.next(); if (elementsOrSeparator.is(Tree.Kind.VARIABLE_DECLARATION)) { var variableDeclaration = (VariableDeclarationTree) elementsOrSeparator; @@ -54,7 +70,7 @@ public void visitConstDeclaration(ConstantDeclarationTree tree) { @Override public void visitVariableDeclaration(VariableDeclarationTree tree) { -// Pattern.compile("=\\s*+([^\\\\ &;#,|]+)"); + // Pattern.compile("=\\s*+([^\\\\ &;#,|]+)"); System.out.println("BBBB variable declaration: " + tree.identifier() + " kind " + tree.getKind()); if (tree.initValue() instanceof LiteralTree literalTree) { detectSecret(tree.identifier().text(), CheckUtils.trimQuotes(literalTree.value()), literalTree); @@ -73,16 +89,16 @@ public void visitFunctionCall(FunctionCallTree functionCall) { .filter(constantName -> constantName.is(Tree.Kind.REGULAR_STRING_LITERAL)) .map(LiteralTree.class::cast) .ifPresent(constantName -> { - //TODO remove + // TODO remove System.out.println("DDDD constantName " + constantName.value()); CheckUtils.argumentValue(functionCall, "value", 1) .filter(value -> value.is(Tree.Kind.REGULAR_STRING_LITERAL)) .map(LiteralTree.class::cast) - .ifPresent(value -> { - //TODO remove - System.out.println("DDDD value: " + value.value()); - detectSecret(CheckUtils.trimQuotes(constantName.value()), CheckUtils.trimQuotes(value.value()), value); - }); + .ifPresent(value -> { + // TODO remove + System.out.println("DDDD value: " + value.value()); + detectSecret(CheckUtils.trimQuotes(constantName.value()), CheckUtils.trimQuotes(value.value()), value); + }); }); } @@ -90,8 +106,8 @@ public void visitFunctionCall(FunctionCallTree functionCall) { } private void detectSecret(String identifierName, String secret, Tree tree) { - //TODO remove -// System.out.println("AAAA detectSecret " + identifierName); + // TODO remove + // System.out.println("AAAA detectSecret " + identifierName); getSecretLikeName(identifierName).ifPresent(secretName -> { if (secret.equals("abcdefghijklmnopqrs")) { newIssue(tree, "'%s' detected in this expression, review this potentially hard-coded secret.".formatted(secretName)); diff --git a/php-checks/src/test/java/org/sonar/php/checks/HardCodedSecretCheckTest.java b/php-checks/src/test/java/org/sonar/php/checks/HardCodedSecretCheckTest.java index 71492a6da..eb8b92c70 100644 --- a/php-checks/src/test/java/org/sonar/php/checks/HardCodedSecretCheckTest.java +++ b/php-checks/src/test/java/org/sonar/php/checks/HardCodedSecretCheckTest.java @@ -1,3 +1,22 @@ +/* + * SonarQube PHP Plugin + * Copyright (C) 2010-2024 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ package org.sonar.php.checks; import org.junit.jupiter.api.Test; From 0f3683f0502b860fd504af996e0969a4f9e4ca13 Mon Sep 17 00:00:00 2001 From: Marcin Stachniuk Date: Tue, 5 Nov 2024 10:05:24 +0100 Subject: [PATCH 3/3] Add more AST support --- .../java/org/sonar/php/checks/CheckList.java | 1 + .../php/checks/HardCodedSecretCheck.java | 122 +++++++--- .../sonar/php/checks/utils/CheckUtils.java | 13 +- .../resources/checks/HardCodedSecretCheck.php | 215 +++++++++++++++++- 4 files changed, 313 insertions(+), 38 deletions(-) diff --git a/php-checks/src/main/java/org/sonar/php/checks/CheckList.java b/php-checks/src/main/java/org/sonar/php/checks/CheckList.java index 1c470bb03..d72e63f69 100644 --- a/php-checks/src/main/java/org/sonar/php/checks/CheckList.java +++ b/php-checks/src/main/java/org/sonar/php/checks/CheckList.java @@ -200,6 +200,7 @@ public static List> getGeneralChecks() { HardCodedCredentialsInFunctionCallsCheck.class, HardCodedCredentialsInVariablesAndUrisCheck.class, HardCodedIpAddressCheck.class, + HardCodedSecretCheck.class, HardCodedUriCheck.class, HttpOnlyCheck.class, IdenticalOperandsInBinaryExpressionCheck.class, diff --git a/php-checks/src/main/java/org/sonar/php/checks/HardCodedSecretCheck.java b/php-checks/src/main/java/org/sonar/php/checks/HardCodedSecretCheck.java index a725361ae..6000b6ee2 100644 --- a/php-checks/src/main/java/org/sonar/php/checks/HardCodedSecretCheck.java +++ b/php-checks/src/main/java/org/sonar/php/checks/HardCodedSecretCheck.java @@ -19,7 +19,6 @@ */ package org.sonar.php.checks; -import java.util.Iterator; import java.util.List; import java.util.Optional; import java.util.regex.Matcher; @@ -29,14 +28,20 @@ import org.sonar.check.RuleProperty; import org.sonar.php.checks.utils.CheckUtils; import org.sonar.plugins.php.api.tree.Tree; -import org.sonar.plugins.php.api.tree.declaration.ConstantDeclarationTree; +import org.sonar.plugins.php.api.tree.declaration.ParameterTree; import org.sonar.plugins.php.api.tree.declaration.VariableDeclarationTree; +import org.sonar.plugins.php.api.tree.expression.ArrayPairTree; +import org.sonar.plugins.php.api.tree.expression.AssignmentExpressionTree; +import org.sonar.plugins.php.api.tree.expression.BinaryExpressionTree; import org.sonar.plugins.php.api.tree.expression.ExpandableStringCharactersTree; import org.sonar.plugins.php.api.tree.expression.FunctionCallTree; import org.sonar.plugins.php.api.tree.expression.HeredocStringLiteralTree; import org.sonar.plugins.php.api.tree.expression.LiteralTree; +import org.sonar.plugins.php.api.tree.expression.VariableIdentifierTree; import org.sonar.plugins.php.api.visitors.PHPVisitorCheck; +import static org.sonar.php.checks.utils.CheckUtils.trimQuotes; + @Rule(key = "S6418") public class HardCodedSecretCheck extends PHPVisitorCheck { private static final String DEFAULT_SECRET_WORDS = "api[_.-]?key,auth,credential,secret,token"; @@ -56,59 +61,114 @@ public class HardCodedSecretCheck extends PHPVisitorCheck { private List variablePatterns = null; - @Override - public void visitConstDeclaration(ConstantDeclarationTree tree) { - // TODO remove - for (Iterator it = tree.declarations().elementsAndSeparators(); it.hasNext();) { - Tree elementsOrSeparator = it.next(); - if (elementsOrSeparator.is(Tree.Kind.VARIABLE_DECLARATION)) { - var variableDeclaration = (VariableDeclarationTree) elementsOrSeparator; - visitVariableDeclaration(variableDeclaration); - } - } - } - @Override public void visitVariableDeclaration(VariableDeclarationTree tree) { - // Pattern.compile("=\\s*+([^\\\\ &;#,|]+)"); - System.out.println("BBBB variable declaration: " + tree.identifier() + " kind " + tree.getKind()); if (tree.initValue() instanceof LiteralTree literalTree) { - detectSecret(tree.identifier().text(), CheckUtils.trimQuotes(literalTree.value()), literalTree); + detectSecret(tree.identifier().text(), trimQuotes(literalTree.value()), literalTree); } if (tree.initValue() instanceof HeredocStringLiteralTree heredoc) { for (ExpandableStringCharactersTree heredocLine : heredoc.strings()) { detectSecret(tree.identifier().text(), heredocLine.value(), heredocLine); } } + super.visitVariableDeclaration(tree); } @Override - public void visitFunctionCall(FunctionCallTree functionCall) { - if ("define".equals(CheckUtils.getLowerCaseFunctionName(functionCall))) { - CheckUtils.argumentValue(functionCall, "constant_name", 0) + public void visitFunctionCall(FunctionCallTree tree) { + var functionName = CheckUtils.getLowerCaseFunctionName(tree); + if ("define".equals(functionName)) { + CheckUtils.argumentValue(tree, "constant_name", 0) .filter(constantName -> constantName.is(Tree.Kind.REGULAR_STRING_LITERAL)) .map(LiteralTree.class::cast) .ifPresent(constantName -> { - // TODO remove - System.out.println("DDDD constantName " + constantName.value()); - CheckUtils.argumentValue(functionCall, "value", 1) + CheckUtils.argumentValue(tree, "value", 1) .filter(value -> value.is(Tree.Kind.REGULAR_STRING_LITERAL)) .map(LiteralTree.class::cast) .ifPresent(value -> { - // TODO remove - System.out.println("DDDD value: " + value.value()); - detectSecret(CheckUtils.trimQuotes(constantName.value()), CheckUtils.trimQuotes(value.value()), value); + detectSecret(trimQuotes(constantName.value()), trimQuotes(value.value()), value); }); }); + } else if ("strcasecmp".equals(functionName) || "strcmp".equals(functionName)) { + var string1 = CheckUtils.resolvedArgumentLiteral(tree, "string1", 0); + var string2 = CheckUtils.resolvedArgumentLiteral(tree, "string2", 1); + if (string1.isPresent()) { + var callArg = tree.callArguments().get(1).value(); + if (callArg instanceof VariableIdentifierTree variableIdentifier) { + detectSecret(variableIdentifier.text(), string1.get().value(), string1.get()); + } + } + if (string2.isPresent()) { + var callArg = tree.callArguments().get(0).value(); + if (callArg instanceof VariableIdentifierTree variableIdentifier) { + detectSecret(variableIdentifier.text(), string2.get().value(), string2.get()); + } + } + } else if (tree.callArguments().size() == 2) { + var firstArg = tree.callArguments().get(0).value(); + var secondArg = tree.callArguments().get(1).value(); + if (firstArg instanceof LiteralTree firstLiteralTree) { + if (secondArg instanceof LiteralTree secondLiteralTree) { + detectSecret(firstLiteralTree.value(), secondLiteralTree.value(), secondLiteralTree); + detectSecret(secondLiteralTree.value(), firstLiteralTree.value(), firstLiteralTree); + } + } + } + super.visitFunctionCall(tree); + } + + @Override + public void visitAssignmentExpression(AssignmentExpressionTree tree) { + var variableIdentifier = tree.variable(); + if (variableIdentifier instanceof VariableIdentifierTree identifier) { + var valueTree = tree.value(); + if (valueTree instanceof LiteralTree literalTree) { + detectSecret(trimQuotes(identifier.text()), trimQuotes(literalTree.value()), literalTree); + } + } + super.visitAssignmentExpression(tree); + } + + @Override + public void visitParameter(ParameterTree tree) { + if (tree.initValue() instanceof LiteralTree valueTree) { + detectSecret(tree.variableIdentifier().text(), valueTree.value(), valueTree); + } + super.visitParameter(tree); + } + + @Override + public void visitArrayPair(ArrayPairTree tree) { + if (tree.key() instanceof LiteralTree keyTree) { + if (tree.value() instanceof LiteralTree valueTree) { + detectSecret(trimQuotes(keyTree.value()), trimQuotes(valueTree.value()), valueTree); + } + } + super.visitArrayPair(tree); + } + + @Override + public void visitBinaryExpression(BinaryExpressionTree tree) { + var leftOperant = tree.leftOperand(); + var rightOperant = tree.rightOperand(); + if (rightOperant instanceof LiteralTree secretValueTree) { + if (leftOperant instanceof VariableIdentifierTree variableTree) { + detectSecret(variableTree.text(), secretValueTree.value(), rightOperant); + } + } + if (leftOperant instanceof LiteralTree secretValueTree) { + if (rightOperant instanceof VariableIdentifierTree variableTree) { + detectSecret(variableTree.text(), secretValueTree.value(), leftOperant); + } } - super.visitFunctionCall(functionCall); + super.visitBinaryExpression(tree); } - private void detectSecret(String identifierName, String secret, Tree tree) { - // TODO remove - // System.out.println("AAAA detectSecret " + identifierName); - getSecretLikeName(identifierName).ifPresent(secretName -> { + private void detectSecret(String identifierName, String secretValue, Tree tree) { + var identifier = trimQuotes(identifierName); + var secret = trimQuotes(secretValue); + getSecretLikeName(identifier).ifPresent(secretName -> { if (secret.equals("abcdefghijklmnopqrs")) { newIssue(tree, "'%s' detected in this expression, review this potentially hard-coded secret.".formatted(secretName)); } diff --git a/php-checks/src/main/java/org/sonar/php/checks/utils/CheckUtils.java b/php-checks/src/main/java/org/sonar/php/checks/utils/CheckUtils.java index 0af051557..153635458 100644 --- a/php-checks/src/main/java/org/sonar/php/checks/utils/CheckUtils.java +++ b/php-checks/src/main/java/org/sonar/php/checks/utils/CheckUtils.java @@ -379,8 +379,10 @@ public static Optional argument(FunctionCallTree call, String } public static Optional resolvedArgumentLiteral(FunctionCallTree call, String name, int position) { - return argumentValue(call, name, position).map(CheckUtils::assignedValue) - .filter(LiteralTree.class::isInstance).map(LiteralTree.class::cast); + return argumentValue(call, name, position) + .map(CheckUtils::assignedValue) + .filter(LiteralTree.class::isInstance) + .map(LiteralTree.class::cast); } public static Optional argumentValue(FunctionCallTree call, String name, int position) { @@ -413,6 +415,13 @@ public static ExpressionTree assignedValue(ExpressionTree tree) { return tree; } + public static Optional assignedValueAsLiteralTree(ExpressionTree tree) { + if (assignedValue(tree) instanceof LiteralTree literalTree) { + return Optional.of(literalTree); + } + return Optional.empty(); + } + public static Optional uniqueAssignedValue(VariableIdentifierTree tree) { Symbol symbol = ((VariableIdentifierTreeImpl) tree).symbol(); if (symbol != null) { diff --git a/php-checks/src/test/resources/checks/HardCodedSecretCheck.php b/php-checks/src/test/resources/checks/HardCodedSecretCheck.php index fb629c588..6ec1125ab 100644 --- a/php-checks/src/test/resources/checks/HardCodedSecretCheck.php +++ b/php-checks/src/test/resources/checks/HardCodedSecretCheck.php @@ -19,7 +19,11 @@ abcdefghijklmnopqrs END; -//TODO how to detect this? +// FN it is literal tree and contains new lines +const TOKEN_NOWDOC = <<<'EOD' +abcdefghijklmnopqrs +EOD; + define("AUTH", "abcdefghijklmnopqrs"); // Noncompliant define("credential", "abcdefghijklmnopqrs"); // Noncompliant @@ -27,7 +31,7 @@ define("namespace\level\Token", "abcdefghijklmnopqrs"); // Noncompliant -// Variables declarations +// Variables declarations (as AssignmentExpression) $var_ok = "abcdefghijklmnopqrs"; // Compliant $oauth = "abcdefghijklmnopqrs"; // Noncompliant @@ -36,18 +40,219 @@ function do_something(): void { $a = "abcdefghijklmnopqrs"; // Compliant - $api_app = "abcdefghijklmnopqrs"; // Noncompliant + $apikey = "abcdefghijklmnopqrs"; // Noncompliant } // Class Constants (as Variables in AST) -class ContainsConstrants { +class ContainsConstrants +{ // Noncompliant@+1 const Token = "abcdefghijklmnopqrs"; // ^^^^^^^^^^^^^^^^^^^^^ - const API_SOMETHING_KEY = "abcdefghijklmnopqrs"; // Noncompliant + const API_KEY = "abcdefghijklmnopqrs"; // Noncompliant const AUTH = "abcdefghijklmnopqrs"; // Noncompliant const OK_CONSTANT = "abcdefghijklmnopqrs"; // Compliant + + // Assignments + function doSomething() + { + $someSecret = ""; + $someSecret = "abcdefghijklmnopqrs"; // Noncompliant + + } } +// Class Properties +class ContainsProperties +{ + public $passed = 'abcdefghijklmnopqrs'; + public $apikey = 'abcdefghijklmnopqrs'; // Noncompliant + public string $api_key = "abcdefghijklmnopqrs"; // Noncompliant + + protected $credential = "abcdefghijklmnopqrs"; // Noncompliant + private $private_credential = "abcdefghijklmnopqrs"; // Noncompliant + static $static_credential = "abcdefghijklmnopqrs"; // Noncompliant + + public function __construct( + public readonly string $auth = "abcdefghijklmnopqrs", // Noncompliant + protected $protected_auth = "abcdefghijklmnopqrs", // Noncompliant + private $private_auth = "abcdefghijklmnopqrs", // Noncompliant + $oauth = "abcdefghijklmnopqrs", // Noncompliant + $empty_auth + ) + { + } + +} + +// Object +$object = (object)[ + 'propertyOne' => 'abcdefghijklmnopqrs', + 'secret' => 'abcdefghijklmnopqrs', // Noncompliant + "token" => "abcdefghijklmnopqrs" // Noncompliant +]; + +// Array +$array = array( + 'passed' => 'abcdefghijklmnopqrs', + 'auth' => 'abcdefghijklmnopqrs', // Noncompliant + "credential" => "abcdefghijklmnopqrs" // Noncompliant +); + +function compareStrings($auth) +{ + if ($auth == null) + { + return; + } + if ($auth == "") + { + return; + } + if ($auth == "X") + { + return; + } + if ($auth == "abcdefghijklmnopqrs") // Noncompliant + { + echo $auth; + } + if ("abcdefghijklmnopqrs" == $auth) // Noncompliant + { + echo $auth; + } + if ($auth === "abcdefghijklmnopqrs") // Noncompliant + { + echo $auth; + } + if ("abcdefghijklmnopqrs" === $auth) // Noncompliant + { + echo $auth; + } + if ($auth <=> "abcdefghijklmnopqrs") // Noncompliant + { + echo $auth; + } + if ("abcdefghijklmnopqrs" <=> $auth) // Noncompliant + { + echo $auth; + } + if (PASSED == $auth) // FN unable to resolve the value + { + echo $auth; + } + if ($auth == PASSED) // FN unable to resolve the value + { + echo $auth; + } + + // case sensitive + if (strcmp($auth, "abcdefghijklmnopqrs")) // Noncompliant + { + echo $auth; + } + if (strcmp("abcdefghijklmnopqrs", $auth)) // Noncompliant + { + echo $auth; + } + if (strcmp($auth, PASSED)) // FN unable to resolve the value + { + echo $auth; + } + if (strcmp(PASSED, $auth)) // FN unable to resolve the value + { + echo $auth; + } + if (strcmp(null, $auth)) + { + echo $auth; + } + if (strcmp("", $auth)) + { + echo $auth; + } + if (strcmp("X", $auth)) + { + echo $auth; + } + + // case insensitive + if (strcasecmp($auth, "abcdefghijklmnopqrs")) // Noncompliant + { + echo $auth; + } + if (strcasecmp("abcdefghijklmnopqrs", $auth)) // Noncompliant + { + echo $auth; + } + if (strcasecmp($auth, PASSED)) // FN unable to resolve the value + { + echo $auth; + } + if (strcasecmp(PASSED, $auth)) // FN unable to resolve the value + { + echo $auth; + } + if (strcasecmp(null, $auth)) + { + echo $auth; + } + if (strcasecmp("", $auth)) + { + echo $auth; + } + if (strcasecmp("X", $auth)) + { + echo $auth; + } +} + +function some2ArgsFunction($arg1, $arg2) +{ + // do nothing +} + +// When a function call has two arguments potentially containing String, we report an issue the same way we would with a variable declaration +function callSome2ArgsFunction() +{ + some2ArgsFunction("secret", "abcdefghijklmnopqrs"); // Noncompliant + some2ArgsFunction("abcdefghijklmnopqrs", "secret"); // Noncompliant + some2ArgsFunction('abcdefghijklmnopqrs', 'secret'); // Noncompliant + some2ArgsFunction("secret", PASSED); // FN unable to resolve the value + some2ArgsFunction(PASSED, "secret"); // FN unable to resolve the value + some2ArgsFunction("secret", "X"); + some2ArgsFunction("secret", ""); + some2ArgsFunction("secret", null); + some2ArgsFunction("secret", 42); + some2ArgsFunction("secret", 'auth'); + some2ArgsFunction("X", "secret"); + some2ArgsFunction("", "secret"); + some2ArgsFunction(null, "secret"); + some2ArgsFunction(42, "secret"); + some2ArgsFunction('auth', "secret"); +} + +function callSome2ArgsMethod() +{ + $box = new Box(); + $box->setProperty("secret", "abcdefghijklmnopqrs"); // Noncompliant + $box->setProperty("abcdefghijklmnopqrs", "secret"); // Noncompliant + $box->setProperty('abcdefghijklmnopqrs', 'secret'); // Noncompliant + $box->setProperty("secret", PASSED); // FN unable to resolve the value + $box->setProperty(PASSED, "secret"); // FN unable to resolve the value + $box->setProperty("secret", "X"); + $box->setProperty("secret", ""); + $box->setProperty("secret", null); + $box->setProperty("secret", 42); + $box->setProperty("secret", 'auth'); +} + +class Box +{ + function setProperty($arg1, $arg2) + { + // do nothing + } +}