Skip to content

Java: CWE-079 Query to detect XSS with JavaServer Faces (JSF) #6393

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

Merged
merged 13 commits into from
Sep 14, 2021

Conversation

luchua-bc
Copy link
Contributor

JavaServer Faces (JSF) is a Java-based MVC web application framework intended to simplify development integration of web-based user interfaces. JavaServer Faces is a standardized display technology for component-based UI development, which was formalized through the Java Community Process and is a part of the Java EE platform.

JSF UI components are rendered as markup languages such as HTML and XML through a JSF renderer. User inputs that don't have proper sanitization and are directly written to a JSF renderer make the website vulnerable to cross-site scripting.

The query detects unsafe usage of JSF renderer that are vulnerable to XSS attacks. Please consider to merge the PR. Thanks.

@luchua-bc luchua-bc requested a review from a team as a code owner August 1, 2021 18:45
@github-actions
Copy link
Contributor

github-actions bot commented Aug 1, 2021

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged. The following differences were found:

java

Generated file changes for java

  • Changes to framework-coverage-java.rst:
-    Java extensions,"``javax.*``, ``jakarta.*``",22,540,18,,,,,1,1,2
+    Java extensions,"``javax.*``, ``jakarta.*``",42,540,22,,,4,,1,1,2
-    Totals,,84,2465,296,13,6,6,107,33,1,66
+    Totals,,104,2465,300,13,6,10,107,33,1,66
  • Changes to framework-coverage-java.csv:
+ jakarta.faces.context,2,10,,,,,,,,,,,,,,2,10,,
+ javax.faces.context,2,10,,,,,,,,,,,,,,2,10,,

luchua-bc and others added 10 commits September 14, 2021 11:47
I don't think we need this: there are lots of possible XSS vectors; we don't need to enumerate every one in the qhelp file.
Per previous commit, no need for a top-level JSF example
JSP and Servlet already shared this logic; might as well add JSF into the same mechanism.
These access application-owned resources AFAICT
@smowton
Copy link
Contributor

smowton commented Sep 14, 2021

@luchua-bc I've polished this up to what I'd consider main query-pack standard -- you might want to take a look at the new commits to see what's changed which would be welcome in future for any query targeting the main (non-experimental) suite.

@luchua-bc
Copy link
Contributor Author

Thanks @smowton a lot for refactoring the query into the main (non-experimental) suite. Those separate commits are crystal clear. I will make sure future queries follow the best practices:

  • Only include stub methods and constants being used by the query
  • Merge with configuration and test cases of the same kind, e.g. servlet and JSF
  • Put qll libraries in java/ql/lib instead of java/ql/src
  • Add reverse import from the external flow library
  • Create a change note

Copy link
Contributor

@aschackmull aschackmull left a comment

Choose a reason for hiding this comment

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

LGTM.

@github-actions
Copy link
Contributor

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged. The following differences were found:

java

Generated file changes for java

  • Changes to framework-coverage-java.rst:
-    Java extensions,"``javax.*``, ``jakarta.*``",40,552,27,,,,,1,1,2
+    Java extensions,"``javax.*``, ``jakarta.*``",54,552,31,,,4,,1,1,2
-    Totals,,102,3554,398,13,6,6,107,33,1,66
+    Totals,,116,3554,402,13,6,10,107,33,1,66
  • Changes to framework-coverage-java.csv:
+ jakarta.faces.context,2,7,,,,,,,,,,,,,,,,,,2,7,,
+ javax.faces.context,2,7,,,,,,,,,,,,,,,,,,2,7,,

Copy link
Contributor

@smowton smowton left a comment

Choose a reason for hiding this comment

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

Proxying @aschackmull's approval

@smowton smowton merged commit 6cff0d0 into github:main Sep 14, 2021
@luchua-bc luchua-bc deleted the java/xss-jsf branch September 14, 2021 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants