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

Updates to handle string-building taint with invokedynamic concatenation in JDK > 8 #713

Merged
merged 2 commits into from
Jan 9, 2024

Conversation

jbindel
Copy link
Contributor

@jbindel jbindel commented Aug 18, 2023

Pull Request #690 adds handling of JDK > 8 string concatenation with invokedynamic and makeConcatWithConstants. There are some cases it does not handle such as concat of long or double values. This pull request address the parameter offsets affected by long and double, which take two spots on the stack.

It also fixes issues where taint on a stack object is modified when we do not want that.

In order to test this change, we need some sample classes built with a JDK > 8, and using JDK11 on the current findsecbugs-samples-java introduces errors in the unit tests so there is a build submodule that builds specific classes to a JDK 11 target.

John Bindel added 2 commits August 18, 2023 16:14
…long and double.

The long and double primitive type parameters occupy two stack registers so we check the locations of stack parameters that can modify taint status. Any reference types as well as primitive char will propagate taint to the resulting String.

Do not add a mutable stack index to the TaintMethodConfig because we don't have a mutable object on the stack in the case of the invokedynamic call.
In order to test invokedynamic string concatenation handling, we need to build some sample code with a later JDK so use Java 11 for the compiler target in a new build submodule. We cannot currently switch the existing findsecbugs-samples-java to Java 11 without breaking tests, and a new module is added just for the Java 11 classes we want to test.

Added setter for FindSecBugsGlobalConfig.workaroundVisitInvokeDynamic that the unit test uses.
@jbindel jbindel changed the title Updates to invokedynamic string concatenation to handle more cases Updates to handle string-building taint with invokedynamic concatenation in JDK > 8 Aug 28, 2023
Copy link
Member

@h3xstream h3xstream left a comment

Choose a reason for hiding this comment

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

Thanks for the contributions
I will do some Java 11 specific tests before the release.

@h3xstream h3xstream merged commit 05c71d8 into find-sec-bugs:master Jan 9, 2024
2 checks passed
@jbindel
Copy link
Contributor Author

jbindel commented Feb 8, 2024

To enable handling of Java 9+ string concatenation, we have to set the system property findsecbugs.taint.workaroundvisitinvokedynamic to true. It may be worthwhile to enable this behavior by default as most people may not discover the setting, and Java 8 is becoming less common.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants