hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-24 04:36:35 +01:00
Code Issues Packages Projects Releases Wiki Activity

Match attribute name to reduce FP

Browse Source
This commit is contained in:
luchua-bc
2022-02-11 23:53:31 +00:00
parent e3d0e9f083
commit 78630f25dd
3 changed files with 80 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

46
java/ql/src/experimental/Security/CWE/CWE-073/JFinalController.qll
View File

@@ -38,6 +38,34 @@ class SetRequestAttributeMethod extends Method {
} }
} }
/**
* Holds if the result of an attribute getter call is from a method invocation of remote attribute setter.
* Only values received from remote flow source is to be checked by the query.
*/
predicate isGetAttributeFromRemoteSource(Expr expr) {
exists(MethodAccess gma, MethodAccess sma |
(
gma.getMethod() instanceof GetSessionAttributeMethod and
sma.getMethod() instanceof SetSessionAttributeMethod
or
gma.getMethod() instanceof GetRequestAttributeMethod and
sma.getMethod() instanceof SetRequestAttributeMethod
) and
expr = gma and
gma.getArgument(0).(CompileTimeConstantExpr).getStringValue() =
sma.getArgument(0).(CompileTimeConstantExpr).getStringValue() and
gma.getEnclosingCallable() = sma.getEnclosingCallable() and
TaintTracking::localExprTaint(any(RemoteFlowSource rs).asExpr(), sma.getArgument(1))
)
}
/** Remote flow source of JFinal request or session attribute getters. */
private class JFinalRequestSource extends RemoteFlowSource {
JFinalRequestSource() { isGetAttributeFromRemoteSource(this.asExpr()) }
override string getSourceType() { result = "JFinal session or request attribute source" }
}
/** Source model of remote flow source with `JFinal`. */ /** Source model of remote flow source with `JFinal`. */
private class JFinalControllerSource extends SourceModelCsv { private class JFinalControllerSource extends SourceModelCsv {
override predicate row(string row) { override predicate row(string row) {
@@ -58,21 +86,3 @@ private class JFinalControllerSource extends SourceModelCsv {
] ]
} }
} }
/** `JFinal` data model related to session and request attribute operations. */
private class JFinalDataModel extends SummaryModelCsv {
override predicate row(string row) {
row =
[
"com.jfinal.core;Controller;true;setSessionAttr;;;Argument[0];MapKey of SyntheticField[com.jfinal.core.Controller.session] of Argument[-1];value",
"com.jfinal.core;Controller;true;setSessionAttr;;;Argument[1];MapValue of SyntheticField[com.jfinal.core.Controller.session] of Argument[-1];value",
"com.jfinal.core;Controller;true;getSessionAttr;;;MapValue of SyntheticField[com.jfinal.core.Controller.session] of Argument[-1];ReturnValue;value",
"com.jfinal.core;Controller;true;set" + ["", "Attr"] +
";;;Argument[0];MapKey of SyntheticField[com.jfinal.core.Controller.request] of Argument[-1];value",
"com.jfinal.core;Controller;true;set" + ["", "Attr"] +
";;;Argument[1];MapValue of SyntheticField[com.jfinal.core.Controller.request] of Argument[-1];value",
"com.jfinal.core;Controller;true;get" + ["Attr", "AttrForStr"] +
";;;MapValue of SyntheticField[com.jfinal.core.Controller.request] of Argument[-1];ReturnValue;value"
]
}
}

28
java/ql/test/experimental/query-tests/security/CWE-073/FilePathInjection.expected
View File

@@ -1,36 +1,20 @@
edges edges
| FilePathInjection.java:21:21:21:34 | getPara(...) : String | FilePathInjection.java:26:47:26:59 | finalFilePath | | FilePathInjection.java:21:21:21:34 | getPara(...) : String | FilePathInjection.java:26:47:26:59 | finalFilePath |
| FilePathInjection.java:64:21:64:34 | getPara(...) : String | FilePathInjection.java:65:31:65:38 | savePath : String |
| FilePathInjection.java:65:3:65:39 | this <.method> [post update] [com.jfinal.core.Controller.session, <map.value>] : String | FilePathInjection.java:66:29:66:55 | this <.method> [com.jfinal.core.Controller.session, <map.value>] : String |
| FilePathInjection.java:65:31:65:38 | savePath : String | FilePathInjection.java:65:3:65:39 | this <.method> [post update] [com.jfinal.core.Controller.session, <map.value>] : String |
| FilePathInjection.java:66:29:66:55 | getSessionAttr(...) : String | FilePathInjection.java:72:47:72:59 | finalFilePath | | FilePathInjection.java:66:29:66:55 | getSessionAttr(...) : String | FilePathInjection.java:72:47:72:59 | finalFilePath |
| FilePathInjection.java:66:29:66:55 | this <.method> [com.jfinal.core.Controller.session, <map.value>] : String | FilePathInjection.java:66:29:66:55 | getSessionAttr(...) : String |
| FilePathInjection.java:87:21:87:34 | getPara(...) : String | FilePathInjection.java:88:24:88:31 | savePath : String |
| FilePathInjection.java:88:3:88:32 | this <.method> [post update] [com.jfinal.core.Controller.request, <map.value>] : String | FilePathInjection.java:89:29:89:48 | this <.method> [com.jfinal.core.Controller.request, <map.value>] : String |
| FilePathInjection.java:88:24:88:31 | savePath : String | FilePathInjection.java:88:3:88:32 | this <.method> [post update] [com.jfinal.core.Controller.request, <map.value>] : String |
| FilePathInjection.java:89:29:89:48 | getAttr(...) : String | FilePathInjection.java:95:47:95:59 | finalFilePath | | FilePathInjection.java:89:29:89:48 | getAttr(...) : String | FilePathInjection.java:95:47:95:59 | finalFilePath |
| FilePathInjection.java:89:29:89:48 | this <.method> [com.jfinal.core.Controller.request, <map.value>] : String | FilePathInjection.java:89:29:89:48 | getAttr(...) : String | | FilePathInjection.java:205:17:205:44 | getParameter(...) : String | FilePathInjection.java:209:24:209:31 | filePath |
| FilePathInjection.java:159:17:159:44 | getParameter(...) : String | FilePathInjection.java:163:24:163:31 | filePath |
nodes nodes
| FilePathInjection.java:21:21:21:34 | getPara(...) : String | semmle.label | getPara(...) : String | | FilePathInjection.java:21:21:21:34 | getPara(...) : String | semmle.label | getPara(...) : String |
| FilePathInjection.java:26:47:26:59 | finalFilePath | semmle.label | finalFilePath | | FilePathInjection.java:26:47:26:59 | finalFilePath | semmle.label | finalFilePath |
| FilePathInjection.java:64:21:64:34 | getPara(...) : String | semmle.label | getPara(...) : String |
| FilePathInjection.java:65:3:65:39 | this <.method> [post update] [com.jfinal.core.Controller.session, <map.value>] : String | semmle.label | this <.method> [post update] [com.jfinal.core.Controller.session, <map.value>] : String |
| FilePathInjection.java:65:31:65:38 | savePath : String | semmle.label | savePath : String |
| FilePathInjection.java:66:29:66:55 | getSessionAttr(...) : String | semmle.label | getSessionAttr(...) : String | | FilePathInjection.java:66:29:66:55 | getSessionAttr(...) : String | semmle.label | getSessionAttr(...) : String |
| FilePathInjection.java:66:29:66:55 | this <.method> [com.jfinal.core.Controller.session, <map.value>] : String | semmle.label | this <.method> [com.jfinal.core.Controller.session, <map.value>] : String |
| FilePathInjection.java:72:47:72:59 | finalFilePath | semmle.label | finalFilePath | | FilePathInjection.java:72:47:72:59 | finalFilePath | semmle.label | finalFilePath |
| FilePathInjection.java:87:21:87:34 | getPara(...) : String | semmle.label | getPara(...) : String |
| FilePathInjection.java:88:3:88:32 | this <.method> [post update] [com.jfinal.core.Controller.request, <map.value>] : String | semmle.label | this <.method> [post update] [com.jfinal.core.Controller.request, <map.value>] : String |
| FilePathInjection.java:88:24:88:31 | savePath : String | semmle.label | savePath : String |
| FilePathInjection.java:89:29:89:48 | getAttr(...) : String | semmle.label | getAttr(...) : String | | FilePathInjection.java:89:29:89:48 | getAttr(...) : String | semmle.label | getAttr(...) : String |
| FilePathInjection.java:89:29:89:48 | this <.method> [com.jfinal.core.Controller.request, <map.value>] : String | semmle.label | this <.method> [com.jfinal.core.Controller.request, <map.value>] : String |
| FilePathInjection.java:95:47:95:59 | finalFilePath | semmle.label | finalFilePath | | FilePathInjection.java:95:47:95:59 | finalFilePath | semmle.label | finalFilePath |
| FilePathInjection.java:159:17:159:44 | getParameter(...) : String | semmle.label | getParameter(...) : String | | FilePathInjection.java:205:17:205:44 | getParameter(...) : String | semmle.label | getParameter(...) : String |
| FilePathInjection.java:163:24:163:31 | filePath | semmle.label | filePath | | FilePathInjection.java:209:24:209:31 | filePath | semmle.label | filePath |
subpaths subpaths
#select #select
| FilePathInjection.java:26:47:26:59 | finalFilePath | FilePathInjection.java:21:21:21:34 | getPara(...) : String | FilePathInjection.java:26:47:26:59 | finalFilePath | External control of file name or path due to $@. | FilePathInjection.java:21:21:21:34 | getPara(...) | user-provided value | | FilePathInjection.java:26:47:26:59 | finalFilePath | FilePathInjection.java:21:21:21:34 | getPara(...) : String | FilePathInjection.java:26:47:26:59 | finalFilePath | External control of file name or path due to $@. | FilePathInjection.java:21:21:21:34 | getPara(...) | user-provided value |
| FilePathInjection.java:72:47:72:59 | finalFilePath | FilePathInjection.java:64:21:64:34 | getPara(...) : String | FilePathInjection.java:72:47:72:59 | finalFilePath | External control of file name or path due to $@. | FilePathInjection.java:64:21:64:34 | getPara(...) | user-provided value | | FilePathInjection.java:72:47:72:59 | finalFilePath | FilePathInjection.java:66:29:66:55 | getSessionAttr(...) : String | FilePathInjection.java:72:47:72:59 | finalFilePath | External control of file name or path due to $@. | FilePathInjection.java:66:29:66:55 | getSessionAttr(...) | user-provided value |
| FilePathInjection.java:95:47:95:59 | finalFilePath | FilePathInjection.java:87:21:87:34 | getPara(...) : String | FilePathInjection.java:95:47:95:59 | finalFilePath | External control of file name or path due to $@. | FilePathInjection.java:87:21:87:34 | getPara(...) | user-provided value | | FilePathInjection.java:95:47:95:59 | finalFilePath | FilePathInjection.java:89:29:89:48 | getAttr(...) : String | FilePathInjection.java:95:47:95:59 | finalFilePath | External control of file name or path due to $@. | FilePathInjection.java:89:29:89:48 | getAttr(...) | user-provided value |
| FilePathInjection.java:163:24:163:31 | filePath | FilePathInjection.java:159:17:159:44 | getParameter(...) : String | FilePathInjection.java:163:24:163:31 | filePath | External control of file name or path due to $@. | FilePathInjection.java:159:17:159:44 | getParameter(...) | user-provided value | | FilePathInjection.java:209:24:209:31 | filePath | FilePathInjection.java:205:17:205:44 | getParameter(...) : String | FilePathInjection.java:209:24:209:31 | filePath | External control of file name or path due to $@. | FilePathInjection.java:205:17:205:44 | getParameter(...) | user-provided value |

46
java/ql/test/experimental/query-tests/security/CWE-073/FilePathInjection.java
View File

@@ -128,6 +128,52 @@ public class FilePathInjection extends Controller {
fos.close(); fos.close();
} }
// GOOD: Upload file to a system path from a request object
public void uploadFile6() throws IOException {
setAttr("uploadDir", "/data/upload_dir/");
String requestUploadDir = getAttr("uploadDir");
File file = getFile("fileParam").getFile();
String finalFilePath = BASE_PATH + requestUploadDir;
FileInputStream fis = new FileInputStream(file);
FileOutputStream fos = new FileOutputStream(finalFilePath);
int i = 0;
do {
byte[] buf = new byte[1024];
i = fis.read(buf);
fos.write(buf);
} while (i != -1);
fis.close();
fos.close();
}
// GOOD: Upload file to a system path from a request object
public void uploadFile7() throws IOException {
String savePath = getPara("dir");
setAttr("uploadDir", savePath);
setAttr("realUploadDir", "/data/upload_dir/");
String requestUploadDir = getAttr("realUploadDir");
File file = getFile("fileParam").getFile();
String finalFilePath = BASE_PATH + requestUploadDir;
FileInputStream fis = new FileInputStream(file);
FileOutputStream fos = new FileOutputStream(finalFilePath);
int i = 0;
do {
byte[] buf = new byte[1024];
i = fis.read(buf);
fos.write(buf);
} while (i != -1);
fis.close();
fos.close();
}
private void readFile(HttpServletResponse resp, File file) { private void readFile(HttpServletResponse resp, File file) {
OutputStream os = null; OutputStream os = null;
FileInputStream fis = null; FileInputStream fis = null;