hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-27 09:45:15 +02:00
Code Issues Packages Projects Releases Wiki Activity

Refactor tests to use InlineExpectationsTest

Browse Source
This commit is contained in:
Tony Torralba
2021-09-02 13:54:11 +02:00
parent 1f7990d6bb
commit f8d1e2ac11
7 changed files with 79 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
java/ql/test/query-tests/security/CWE-807/semmle/tests/ConditionalBypass.expected
View File

@@ -1,25 +0,0 @@
edges
| Test.java:17:26:17:38 | args : String[] | Test.java:25:6:25:21 | ... == ... |
| Test.java:31:6:31:27 | getValue(...) : String | Test.java:31:6:31:43 | equals(...) |
| Test.java:36:6:36:27 | getValue(...) : String | Test.java:36:6:36:36 | ... == ... |
| Test.java:81:6:81:27 | getValue(...) : String | Test.java:81:6:81:36 | ... == ... |
| Test.java:91:6:91:27 | getValue(...) : String | Test.java:91:6:91:36 | ... == ... |
nodes
| Test.java:17:26:17:38 | args : String[] | semmle.label | args : String[] |
| Test.java:25:6:25:21 | ... == ... | semmle.label | ... == ... |
| Test.java:31:6:31:27 | getValue(...) : String | semmle.label | getValue(...) : String |
| Test.java:31:6:31:43 | equals(...) | semmle.label | equals(...) |
| Test.java:36:6:36:27 | getValue(...) : String | semmle.label | getValue(...) : String |
| Test.java:36:6:36:36 | ... == ... | semmle.label | ... == ... |
| Test.java:81:6:81:27 | getValue(...) : String | semmle.label | getValue(...) : String |
| Test.java:81:6:81:36 | ... == ... | semmle.label | ... == ... |
| Test.java:91:6:91:27 | getValue(...) : String | semmle.label | getValue(...) : String |
| Test.java:91:6:91:36 | ... == ... | semmle.label | ... == ... |
subpaths
#select
| Test.java:26:4:26:24 | login(...) | Test.java:17:26:17:38 | args : String[] | Test.java:25:6:25:21 | ... == ... | Sensitive method may not be executed depending on $@, which flows from $@. | Test.java:25:6:25:21 | ... == ... | this condition | Test.java:17:26:17:38 | args | user input |
| Test.java:32:4:32:24 | login(...) | Test.java:31:6:31:27 | getValue(...) : String | Test.java:31:6:31:43 | equals(...) | Sensitive method may not be executed depending on $@, which flows from $@. | Test.java:31:6:31:43 | equals(...) | this condition | Test.java:31:6:31:27 | getValue(...) | user input |
| Test.java:37:4:37:24 | login(...) | Test.java:36:6:36:27 | getValue(...) : String | Test.java:36:6:36:36 | ... == ... | Sensitive method may not be executed depending on $@, which flows from $@. | Test.java:36:6:36:36 | ... == ... | this condition | Test.java:36:6:36:27 | getValue(...) | user input |
| Test.java:39:4:39:30 | reCheckAuth(...) | Test.java:36:6:36:27 | getValue(...) : String | Test.java:36:6:36:36 | ... == ... | Sensitive method may not be executed depending on $@, which flows from $@. | Test.java:36:6:36:36 | ... == ... | this condition | Test.java:36:6:36:27 | getValue(...) | user input |
| Test.java:82:4:82:24 | login(...) | Test.java:81:6:81:27 | getValue(...) : String | Test.java:81:6:81:36 | ... == ... | Sensitive method may not be executed depending on $@, which flows from $@. | Test.java:81:6:81:36 | ... == ... | this condition | Test.java:81:6:81:27 | getValue(...) | user input |
| Test.java:92:4:92:24 | login(...) | Test.java:91:6:91:27 | getValue(...) : String | Test.java:91:6:91:36 | ... == ... | Sensitive method may not be executed depending on $@, which flows from $@. | Test.java:91:6:91:36 | ... == ... | this condition | Test.java:91:6:91:27 | getValue(...) | user input |

1
java/ql/test/query-tests/security/CWE-807/semmle/tests/ConditionalBypass.qlref
View File

@@ -1 +0,0 @@
Security/CWE/CWE-807/ConditionalBypass.ql

0
java/ql/test/query-tests/security/CWE-807/semmle/tests/ConditionalBypassTest.expected Normal file
View File

73
java/ql/test/query-tests/security/CWE-807/semmle/tests/Test.java → java/ql/test/query-tests/security/CWE-807/semmle/tests/ConditionalBypassTest.java
View File

@@ -2,58 +2,45 @@
// http://cwe.mitre.org/data/definitions/807.html
package test.cwe807.semmle.tests;
import java.net.InetAddress;
import java.net.Inet4Address;
import java.net.UnknownHostException;
import javax.servlet.http.Cookie;
import javax.servlet.http.HttpServletRequest;
import org.apache.shiro.SecurityUtils;
import org.apache.shiro.subject.Subject;
class Test {
public static void main(String[] args) throws UnknownHostException {
String user = args[0];
String password = args[1];
String isAdmin = args[3];
class ConditionalBypassTest {
public static void main(HttpServletRequest request) throws Exception {
String user = request.getParameter("user");
String password = request.getParameter("password");
String isAdmin = request.getParameter("isAdmin");
// BAD: login is only executed if isAdmin is false, but isAdmin
// is controlled by the user
if(isAdmin=="false")
if (isAdmin == "false") // $ hasConditionalBypassTest
login(user, password);
Cookie adminCookie = getCookies()[0];
// BAD: login is only executed if the cookie value is false, but the cookie
// is controlled by the user
if(adminCookie.getValue().equals("false"))
if (adminCookie.getValue().equals("false")) // $ hasConditionalBypassTest
login(user, password);
// FALSE POSITIVES: both methods are conditionally executed, but they probably
// both perform the security-critical action
if(adminCookie.getValue()=="false") {
if (adminCookie.getValue() == "false") { // $ SPURIOUS: $ hasConditionalBypassTest
login(user, password);
} else {
reCheckAuth(user, password);
}
// FALSE NEGATIVE: we have no way of telling that the skipped method is sensitive
if(adminCookie.getValue()=="false")
if (adminCookie.getValue() == "false") // $ MISSING: $ hasConditionalBypassTest
doReallyImportantSecurityWork();
// Apache Shiro permissions system
String whatDoTheyWantToDo = args[4];
Subject subject = SecurityUtils.getSubject();
// BAD: permissions decision made using tainted data
if(subject.isPermitted("domain:sublevel:" + whatDoTheyWantToDo))
doIt();
// GOOD: use fixed checks
if(subject.isPermitted("domain:sublevel:whatTheMethodDoes"))
doIt();
InetAddress local = InetAddress.getLocalHost();
// GOOD: reverse DNS on localhost is fine
if (local.getCanonicalHostName().equals("localhost")) {
@@ -63,32 +50,32 @@ class Test {
login(user, password);
}
}
public static void test(String user, String password) {
Cookie adminCookie = getCookies()[0];
// GOOD: login always happens
if(adminCookie.getValue()=="false")
if (adminCookie.getValue() == "false")
login(user, password);
else {
// do something else
login(user, password);
}
}
public static void test2(String user, String password) {
Cookie adminCookie = getCookies()[0];
// BAD: login may happen once or twice
if(adminCookie.getValue()=="false")
if (adminCookie.getValue() == "false") // $ hasConditionalBypassTest
login(user, password);
else {
// do something else
}
login(user, password);
}
public static void test3(String user, String password) {
Cookie adminCookie = getCookies()[0];
if(adminCookie.getValue()=="false")
if (adminCookie.getValue() == "false") // $ hasConditionalBypassTest
login(user, password);
else {
// do something else
@@ -96,35 +83,35 @@ class Test {
return;
}
}
public static void test4(String user, String password) {
Cookie adminCookie = getCookies()[0];
// GOOD: login always happens
if(adminCookie.getValue()=="false") {
if (adminCookie.getValue() == "false") {
login(user, password);
return;
}
// do other things
login(user, password);
return;
}
public static void login(String user, String password) {
// login
}
public static void reCheckAuth(String user, String password) {
// login
}
public static Cookie[] getCookies() {
// get cookies from a servlet
return new Cookie[0];
}
public static void doIt() {}
public static void doReallyImportantSecurityWork() {
// login, authenticate, everything
}

20
java/ql/test/query-tests/security/CWE-807/semmle/tests/ConditionalBypassTest.ql Normal file
View File

@@ -0,0 +1,20 @@
import java
import semmle.code.java.security.ConditionalBypassQuery
import TestUtilities.InlineExpectationsTest
class ConditionalBypassTest extends InlineExpectationsTest {
ConditionalBypassTest() { this = "ConditionalBypassTest" }
override string getARelevantTag() { result = "hasConditionalBypassTest" }
override predicate hasActualResult(Location location, string element, string tag, string value) {
tag = "hasConditionalBypassTest" and
exists(DataFlow::Node src, DataFlow::Node sink, ConditionalBypassFlowConfig conf |
conf.hasFlow(src, sink)
|
sink.getLocation() = location and
element = sink.toString() and
value = ""
)
}
}

9
java/ql/test/query-tests/security/CWE-807/semmle/tests/TaintedPermissionsCheck.expected
View File

@@ -1,8 +1,7 @@
edges
| Test.java:17:26:17:38 | args : String[] | Test.java:50:26:50:64 | ... + ... |
| TaintedPermissionsCheckTest.java:12:19:12:48 | getParameter(...) : String | TaintedPermissionsCheckTest.java:15:27:15:53 | ... + ... |
nodes
| Test.java:17:26:17:38 | args : String[] | semmle.label | args : String[] |
| Test.java:50:26:50:64 | ... + ... | semmle.label | ... + ... |
subpaths
| TaintedPermissionsCheckTest.java:12:19:12:48 | getParameter(...) : String | semmle.label | getParameter(...) : String |
| TaintedPermissionsCheckTest.java:15:27:15:53 | ... + ... | semmle.label | ... + ... |
#select
| Test.java:50:6:50:65 | isPermitted(...) | Test.java:17:26:17:38 | args : String[] | Test.java:50:26:50:64 | ... + ... | Permissions check uses user-controlled $@. | Test.java:17:26:17:38 | args | data |
| TaintedPermissionsCheckTest.java:15:7:15:54 | isPermitted(...) | TaintedPermissionsCheckTest.java:12:19:12:48 | getParameter(...) : String | TaintedPermissionsCheckTest.java:15:27:15:53 | ... + ... | Permissions check uses user-controlled $@. | TaintedPermissionsCheckTest.java:12:19:12:48 | getParameter(...) | data |

25
java/ql/test/query-tests/security/CWE-807/semmle/tests/TaintedPermissionsCheckTest.java Normal file
View File

@@ -0,0 +1,25 @@
// Test case for CWE-807 (Reliance on Untrusted Inputs in a Security Decision)
// http://cwe.mitre.org/data/definitions/807.html
package test.cwe807.semmle.tests;
import javax.servlet.http.HttpServletRequest;
import org.apache.shiro.SecurityUtils;
import org.apache.shiro.subject.Subject;
class TaintedPermissionsCheckTest {
public static void main(HttpServletRequest request) throws Exception {
// Apache Shiro permissions system
String action = request.getParameter("action");
Subject subject = SecurityUtils.getSubject();
// BAD: permissions decision made using tainted data
if (subject.isPermitted("domain:sublevel:" + action))
doIt();
// GOOD: use fixed checks
if (subject.isPermitted("domain:sublevel:whatTheMethodDoes"))
doIt();
}
public static void doIt() {}
}