github / codeql Public

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

Jump to bottom

Java: CWE-400 - Query to detect uncontrolled thread resource consumption #6717

Open

luchua-bc wants to merge 5 commits into github:main from luchua-bc:java/thread-resource-abuse

Contributor

luchua-bc commented Sep 20, 2021

Thread.sleep method is used to pause the execution of current thread for specified time. When it is used to keep several relevant tasks in synchronization and the sleep time is user-controlled data, especially in the web application context, it can be abused to cause all of a server's threads to sleep, leading to denial of service.

The query detects uncontrolled thread resource consumption in Java EE web applications, which checks:

sources of servlets and JSF
remote user input and local container configuration
both the javax and jakarta packages

Please consider to merge the PR. Thanks.

The text was updated successfully, but these errors were encountered:


          Query to detect uncontrolled thread resource consumption

8170f01

luchua-bc requested a review from as a code owner

github-actions bot added documentation Java labels

luchua-bc mentioned this pull request

[Java] CWE-400: Query to detect uncontrolled thread resource consumption github/securitylab#431

Open

1 task

Marcono1234 reviewed

java/ql/src/experimental/Security/CWE/CWE-400/ThreadResourceAbuse.qhelp Outdated Show resolved Hide resolved

java/ql/src/experimental/Security/CWE/CWE-400/ThreadResourceAbuse.qhelp Outdated Show resolved Hide resolved

java/ql/src/experimental/Security/CWE/CWE-400/ThreadResourceAbuse.qhelp Outdated Show resolved Hide resolved


          Update qldoc

2dc38ae

luchua-bc force-pushed the java/thread-resource-abuse branch from 80bb663 to 2dc38ae

Contributor Author

luchua-bc commented Sep 23, 2021

Thanks @Marcono1234 and @intrigus-lgtm for reviewing this PR. I've made requested changes.

Marcono1234 reviewed

Contributor

Marcono1234 left a comment

Thanks for considering the review comments!
I have had a closer look at the implementation of this query now; hopefully these comments are useful as well.

Though as usual, feel free to consider them only as suggestions because I am not a member of this project.

java/ql/src/experimental/Security/CWE/CWE-400/ThreadResourceAbuse.ql Show resolved Hide resolved

java/ql/test/experimental/query-tests/security/CWE-400/ThreadResourceAbuse.java Outdated Show resolved Hide resolved

java/ql/src/experimental/Security/CWE/CWE-400/ThreadResourceAbuse.ql Outdated Show resolved Hide resolved

java/ql/src/experimental/Security/CWE/CWE-400/ThreadResourceAbuse.qhelp Outdated Show resolved Hide resolved

java/ql/src/experimental/Security/CWE/CWE-400/ThreadResourceAbuse.ql Outdated Show resolved Hide resolved

java/ql/src/experimental/Security/CWE/CWE-400/ThreadResourceAbuse.ql Outdated Show resolved Hide resolved

java/ql/src/experimental/Security/CWE/CWE-400/ThreadResourceAbuse.ql Outdated Show resolved Hide resolved


          Update the query

272e4f6

Contributor Author

luchua-bc commented Sep 24, 2021

I've also added a new test case to detect remote source from an HTTP request header such as Retry-After:

codeql/java/ql/test/experimental/query-tests/security/CWE-400/ThreadResourceAbuse.java

Lines 170 to 173 in 272e4f6

    
           protected void doHead2(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { 
        
           	// Get thread pause time from request header 
        
           	String header = request.getHeader("Retry-After"); 
        
           	int retryAfter = Integer.parseInt(header);

github deleted a comment

Marcono1234 reviewed

java/ql/src/experimental/Security/CWE/CWE-400/ThreadResourceAbuse.ql Outdated Show resolved Hide resolved

Marcono1234 reviewed

java/ql/test/experimental/query-tests/security/CWE-400/ThreadResourceAbuse.java Outdated Show resolved Hide resolved

java/ql/test/experimental/query-tests/security/CWE-400/ThreadResourceAbuse.java Show resolved Hide resolved


          Correct the run method and add Math.min check

atorralba reviewed

java/ql/src/experimental/Security/CWE/CWE-400/ThreadPauseSink.qll Outdated Show resolved Hide resolved

java/ql/src/experimental/Security/CWE/CWE-400/ThreadResourceAbuse.ql Outdated Show resolved Hide resolved

java/ql/src/experimental/Security/CWE/CWE-400/ThreadResourceAbuse.ql Show resolved Hide resolved


          Remove local user input and use fluent model

378db7d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment