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

Adding workaround for JDK > 8 invokedynamic tainting #690

Merged
merged 2 commits into from
Jul 26, 2023

Conversation

oxeye-gal
Copy link
Contributor

Hi,

We @oxeye created a possible workaround for this previously discussed issue:

#575

We are not 100% sure that this is the optimal solution for this issue, so we would love to get your feedback on this so we can address your observations.

We also noticed that you have set the target version for fixing this issue for the next version (1.13 scheduled for December), is it still the due date?

Thanks

@oxeye-daniel
Copy link

@h3xstream - is there any update on this?

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.

I didn't investigate deeply this PR but it should fix the missing method for StringConcatFactory / makeConcatWithConstants from Java 9.

String signature = methodId.substring(methodId.indexOf("("), methodId.length());
SignatureParser p = new SignatureParser(signature);
return p.getNumParameters();
}
Copy link
Member

Choose a reason for hiding this comment

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

SignatureParser might not handle long and double parameters (which are double at runtime) but for makeConcatWithConstants it should be fine.

@h3xstream h3xstream merged commit 68628d3 into find-sec-bugs:master Jul 26, 2023
@h3xstream
Copy link
Member

Might fix #701

@jbindel
Copy link
Contributor

jbindel commented Aug 16, 2023

It seems to help greatly for JDK11 bytecodes with the new option turned on. One thing that does not get fixed is taint propagation for makeConcatWithConstants when concatenating a long or double with strings, which I believe is what you noted.

"something" + 1000L is tainted while "something" + Long.toString(1000L) is clean.

@jbindel
Copy link
Contributor

jbindel commented Aug 17, 2023

I have some changes building on this pull request to make TaintConfig work when concatenating long and double values, and I'll make a pull request for them soon.

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.

4 participants