Skip to content

Conversation

pwntester
Copy link
Contributor

CodeQl already models Struts2 unmarhaled objects as a source of untrusted data. Unfortunately, the source is at the object level and their direct and object graph nested fields are not tainted. This PR implements a FieldContent TaintInheriting class for each field in the unmarhaled objects object graph.

@pwntester pwntester requested a review from a team as a code owner July 11, 2023 08:31
@github-actions github-actions bot added the Java label Jul 11, 2023
@atorralba
Copy link
Contributor

Now that we're properly tracking taint through fields, the fact that Struts2ActionSupportClassFieldReadSource was modeled around field reads caused a lot of overlapping paths when a field was read several times in the same method.

By changing it to be a FieldValueNode instead (and in combination with #13817), the situation improves significantly.

@atorralba atorralba force-pushed the java/struts2_source_taint_inheriting branch from 908c1d0 to c239a43 Compare July 27, 2023 08:39
@atorralba atorralba added the depends on internal PR This PR should only be merged in sync with an internal Semmle PR label Jul 27, 2023
Copy link
Contributor

@atorralba atorralba left a comment

Choose a reason for hiding this comment

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

Waiting for the internal PR to be merged, but after that this is good to go!

@atorralba atorralba merged commit 08cba7d into github:main Jul 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
depends on internal PR This PR should only be merged in sync with an internal Semmle PR documentation Java
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants