Skip to content

CodeQL query to check for insecure MaxLengthRequest values in ASP.NET applications#2355

Merged
hvitved merged 6 commits intogithub:masterfrom
cldrn:AspNetMaxRequestLength
Nov 25, 2019
Merged

CodeQL query to check for insecure MaxLengthRequest values in ASP.NET applications#2355
hvitved merged 6 commits intogithub:masterfrom
cldrn:AspNetMaxRequestLength

Conversation

@cldrn
Copy link
Contributor

@cldrn cldrn commented Nov 16, 2019

The directive maxRequestLength sets the limit for the input stream buffering threshold in KB. Attackers can use large requests to cause denial-of-service attacks.

This PR includes a CodeQL query to detect applications with larger values than the recommended one:
https://docs.microsoft.com/en-us/dotnet/api/system.web.configuration.httpruntimesection.maxrequestlength?view=netframework-4.8

@hvitved
Copy link
Contributor

hvitved commented Nov 18, 2019

Closing this PR, as it is a duplicate of #2200.

@hvitved hvitved closed this Nov 18, 2019
@cldrn
Copy link
Contributor Author

cldrn commented Nov 18, 2019

Closed the other issue as I removed that local copy and started doing each PR in their own branch. At this point I have deleted the original branch for that PR. I've addressed the comments in there but can we keep working on this one?

The other PR was on master branch which became a mess when I added others there by mistake. Hence, why I started from stratch with each PR in their own branch.

@hvitved
Copy link
Contributor

hvitved commented Nov 18, 2019

OK, let's close the other PR, and continue with this one.

@hvitved hvitved reopened this Nov 18, 2019
Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

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

Please also copy over the test files from the other PR, i.e.

csharp/ql/test/query-tests/Security Features/CWE-016/ASPNetMaxRequestLength.cs
csharp/ql/test/query-tests/Security Features/CWE-016/ASPNetMaxRequestLength.expected
csharp/ql/test/query-tests/Security Features/CWE-016/ASPNetMaxRequestLength.qlref
csharp/ql/test/query-tests/Security Features/CWE-016/Web.config
csharp/ql/test/query-tests/Security Features/CWE-016/bad/Web.config

@cldrn
Copy link
Contributor Author

cldrn commented Nov 19, 2019

Addressed the pending issues.

Thanks for the feedback. Are security test cases needed for all codeql queries?

@hvitved
Copy link
Contributor

hvitved commented Nov 19, 2019

Are security test cases needed for all codeql queries?

Yes, we like to have test cases for all queries.

@hvitved hvitved merged commit fede9ae into github:master Nov 25, 2019
@hvitved
Copy link
Contributor

hvitved commented Nov 25, 2019

Thanks again for your contribution 🎉

@cldrn
Copy link
Contributor Author

cldrn commented Nov 30, 2019

Thanks for the help getting ready. These were from a real sca I had to do so I thought it would be a great way to get familiar with the framework.

@cldrn cldrn deleted the AspNetMaxRequestLength branch November 30, 2019 03:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants