Skip to content

Add MethodAccessSystemGetProperty predicate #4898

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.

Sign up for GitHub

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

Merged
aschackmull merged 3 commits into from
Jan 5, 2021
Merged

Add MethodAccessSystemGetProperty predicate #4898

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
54950c2
Add MethodAccessSystemGetProperty predicate
JLLeitschuh Jan 2, 2021
028e475
Apply suggestions from code review
JLLeitschuh Jan 4, 2021
ba4a562
Update PrintAst.actual with new test output
JLLeitschuh Jan 5, 2021
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
15 changes: 15 additions & 0 deletions java/ql/src/semmle/code/java/JDK.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,21 @@ class MethodSystemGetProperty extends Method {
}
}

/**
* Any method access to a method named `getProperty` on class `java.lang.System`.
Comment thread
JLLeitschuh marked this conversation as resolved.
Outdated
*/
class MethodAccessSystemGetProperty extends MethodAccess {
MethodAccessSystemGetProperty() { getMethod() instanceof MethodSystemGetProperty }

/**
* Holds true if this is a compile-time constant call for the specified `propertyName`.
Copy link
Copy Markdown
Contributor

@smowton smowton Jan 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Holds true if this is a compile-time constant call for the specified `propertyName`.
* Holds true if this call gets a compile-time constant property `propertyName`.

Copy link
Copy Markdown
Contributor Author

@JLLeitschuh JLLeitschuh Jan 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is resolved.

* Eg. `System.getProperty("user.dir")`.
Comment thread
JLLeitschuh marked this conversation as resolved.
Outdated
*/
predicate hasCompileTimeConstantGetPropertyName(string propertyName) {
this.getArgument(0).(CompileTimeConstantExpr).getStringValue() = propertyName
}
}

/**
* Any method named `exit` on class `java.lang.Runtime` or `java.lang.System`.
*/
Expand Down
3 changes: 2 additions & 1 deletion java/ql/src/semmle/code/java/PrintAst.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,6 @@ class PrintAstConfigurationOverride extends PrintAstConfiguration {
/**
* TWEAK THIS PREDICATE AS NEEDED.
*/
override predicate shouldPrint(Element e, Location l) { super.shouldPrint(e, l) }
override predicate shouldPrint(Element e, Location l) { super.shouldPrint(e, l) and
not l.getFile().getBaseName().matches("SystemGetPropertyCall.java") }
Copy link
Copy Markdown
Contributor

@smowton smowton Jan 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accidentally committed?

Copy link
Copy Markdown
Contributor Author

@JLLeitschuh JLLeitschuh Jan 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, this is intentional. Do you want me to just update the test result?

Copy link
Copy Markdown
Contributor

@aschackmull aschackmull Jan 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, please update the test result instead.

Copy link
Copy Markdown
Contributor Author

@JLLeitschuh JLLeitschuh Jan 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be resolved now.

}
3 changes: 3 additions & 0 deletions java/ql/test/library-tests/JDK/SystemGetPropertyCall.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
| jdk/SystemGetPropertyCall.java:7:9:7:38 | getProperty(...) |
| jdk/SystemGetPropertyCall.java:11:9:11:46 | getProperty(...) |
| jdk/SystemGetPropertyCall.java:15:9:15:45 | getProperty(...) |
10 changes: 10 additions & 0 deletions java/ql/test/library-tests/JDK/SystemGetPropertyCall.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/**
* @name SystemCall
* @description Test the definition of System Get Property
*/

import default
Comment thread
JLLeitschuh marked this conversation as resolved.
Outdated

from MethodAccessSystemGetProperty ma
where ma.hasCompileTimeConstantGetPropertyName("user.dir")
select ma
21 changes: 21 additions & 0 deletions java/ql/test/library-tests/JDK/jdk/SystemGetPropertyCall.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package jdk;

public class SystemGetPropertyCall {
private static final String USER_DIR_PROPERTY = "user.dir";

void a() {
System.getProperty("user.dir");
}

void b() {
System.getProperty("user.dir", "HOME");
}

void c() {
System.getProperty(USER_DIR_PROPERTY);
}

void d() {
System.getProperty("random.property");
}
}