Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SONARPHP-1582 S3330: Only raise for variable cookies #1337

Merged
merged 1 commit into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion php-checks/src/main/java/org/sonar/php/checks/HttpOnlyCheck.java
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public void visitFunctionCall(FunctionCallTree tree) {
createIssueIfHttpOnlyIsFalse(argument.get().value(), tree);
} else if (tree.callArguments().size() != 3) {
// if only 3 argument are defined there is an ambiguity so we don't raise issue
context().newIssue(this, tree.callee(), MESSAGE);
createIssueIfCookieValueIsNotHardcoded(tree);
}
}
if (isSymfonyCookieCreation(tree)) {
Expand Down Expand Up @@ -109,4 +109,16 @@ private void createIssueIfHttpOnlyIsFalse(ExpressionTree argument, FunctionCallT
context().newIssue(this, tree.callee(), MESSAGE).secondary(argument, null);
}
}

private void createIssueIfCookieValueIsNotHardcoded(FunctionCallTree tree) {
Optional<CallArgumentTree> cookieValue = CheckUtils.argument(tree, "value", 1);
if (cookieValue.isEmpty() || isHardcodedOrNullCookieValue(cookieValue.get())) {
return;
}
context().newIssue(this, tree.callee(), MESSAGE);
}

private static boolean isHardcodedOrNullCookieValue(CallArgumentTree cookieValue) {
return cookieValue.value().is(Kind.NULL_LITERAL) || cookieValue.value().is(Kind.REGULAR_STRING_LITERAL);
}
}
11 changes: 11 additions & 0 deletions php-checks/src/test/resources/checks/HttpOnlyCheck.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php


setcookie($name, $value, $expire, $path, $domain, true, false); // Noncompliant {{Make sure creating this cookie without the "httpOnly" flag is safe here.}}
//^^^^^^^^^ ^^^^^ <
setrawcookie($name, $value, $expire, $path, $domain, true, false); // Noncompliant {{Make sure creating this cookie without the "httpOnly" flag is safe here.}}
Expand All @@ -13,6 +14,16 @@
setcookie($name, $value, $expire, $path); // Noncompliant
setcookie($name, $value); // Noncompliant


setcookie("tokenIdentity"); // Compliant; cookie value is empty by default
setcookie("tokenIdentity", path:"/", value:'foo', expires_or_options:time() - 42000); // Compliant; cookie value is hardcoded
setcookie(session_name(), '', time() - 3600, '/'); // Compliant; cookie value is hardcoded
setcookie(session_name(), "", time() - 3600, '/'); // Compliant; cookie value is hardcoded
\setcookie(\session_name(), null, -1, \OC::$WEBROOT ? : '/'); // Compliant; cookie value is null
setcookie("tokenIdentity", 'foo', time() - 42000, "/"); // Compliant; cookie value is hardcoded
setcookie("tokenIdentity", "foo", time() - 42000, "/"); // Compliant; cookie value is hardcoded


setrawcookie($name, $value, $expire, $path, $domain, true, foo(false));
setrawcookie($name, $value, $expire, $path, $domain, true, true);
setrawcookie($name, $value, $expire, $path, $domain, false); // Noncompliant
Expand Down
Loading