Java: Add CodeQL support and tests for Micronaut framework - #5
anurag6569201 wants to merge 1 commit into
Conversation
Source PR: github#21387 Source head: 16e9b22
⛔ Shipwright · BlockedRecommendation: do not merge PR #5 · Tier
Findings (8)
Fireworks usage: 54,562 input · 1,033 output · 55,595 total tokens · $0.0127 · 18s · 0 fix iteration(s) Open the Shipwright check for full evidence and the audit bundle. Use |
![shipwright-agent[bot]](https://avatars.githubusercontent.com/u/140162738?s=60&v=4){kind=small}
| /** Holds if the parameter should not be considered a direct source of taint. */ | ||
| predicate isNotDirectlyTaintedInput() { | ||
| this.getType().(RefType).getAnAncestor().hasQualifiedName("io.micronaut.http", "HttpResponse") | ||
| or |
There was a problem hiding this comment.
Shipwright · CRITICAL
The exclusion predicate uses getAnAncestor() on RefType, which only checks supertypes, not implemented interfaces.
Impact: The exclusion predicate uses getAnAncestor() on RefType, which only checks supertypes, not implemented interfaces. Parameters typed as java.security.Principal, java.util.Locale, java.io.Reader, or java.io.InputStream via interface types will not be excluded and will be incorrectly tainted, causing false positives in every query consuming FlowSources.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| this.getType().(RefType).getAnAncestor() instanceof TypeInputStream | ||
| or | ||
| this.getType().(RefType).getAnAncestor().hasQualifiedName("java.io", "Reader") | ||
| or |
There was a problem hiding this comment.
Shipwright · CRITICAL
MicronautRequestMappingParameter.isTaintedInput defaults every unannotated controller method parameter to tainted unless it matches a narrow exclusion list.
Impact: MicronautRequestMappingParameter.isTaintedInput defaults every unannotated controller method parameter to tainted unless it matches a narrow exclusion list. Framework-injected types not in the exclusion list (e.g., Authentication, custom Principal subclasses, BindingResult, HttpSession, reactive contexts) will be treated as remote user input, producing false-positive taint flows and potentially masking real findings…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| } | ||
|
|
||
| /** A parameter of a Micronaut WebSocket message handler that receives user-controlled data. */ | ||
| class MicronautWebSocketParameter extends Parameter { |
There was a problem hiding this comment.
Shipwright · CRITICAL
MicronautWebSocketParameter treats every parameter of OnOpen, OnMessage, and OnClose handlers as remote taint except CloseReason and WebSocketSession.
Impact: MicronautWebSocketParameter treats every parameter of OnOpen, OnMessage, and OnClose handlers as remote taint except CloseReason and WebSocketSession. OnOpen handlers commonly receive framework-injected WebSocketSession, HttpRequest, or Principal parameters; OnClose handlers receive CloseReason. Any other framework type (e.g., WebSocketBroadcaster, WebSocketSession subclasses) becomes a false taint source, and OnOpe…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| /** | ||
| * Provides classes for identifying Micronaut HTTP controllers and their request handling methods. | ||
| */ | ||
| overlay[local?] |
There was a problem hiding this comment.
Shipwright · HIGH
The new Micronaut framework modules are declared as overlay[local?] modules.
Impact: The new Micronaut framework modules are declared as overlay[local?] modules. If this syntax is not supported by the target CodeQL distribution or is misconfigured, the modules may be silently ignored or fail to compile, leaving the advertised Micronaut modeling inactive without any test or CI evidence in the provided check index.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| pack: codeql/java-all | ||
| extensible: sourceModel | ||
| data: | ||
| - ["io.micronaut.http", "HttpRequest", True, "getBody", "", "", "ReturnValue", "remote", "manual"] |
There was a problem hiding this comment.
Shipwright · HIGH
The source model marks HttpRequest.getContentLength() and getMethodName() as remote taint sources.
Impact: The source model marks HttpRequest.getContentLength() and getMethodName() as remote taint sources. These return a long and a String derived from request metadata, not attacker-controlled content. Treating them as tainted will create false flows into numeric and string sinks and dilute query precision.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| - addsTo: | ||
| pack: codeql/java-all | ||
| extensible: summaryModel | ||
| data: |
There was a problem hiding this comment.
Shipwright · HIGH
The summary model for UriBuilder.expand(Map) propagates taint from Argument[0].MapValue to ReturnValue, but the stub and real Micronaut API use Map<String, ?
Impact: The summary model for UriBuilder.expand(Map) propagates taint from Argument[0].MapValue to ReturnValue, but the stub and real Micronaut API use Map<String, ? super Object>. If the CodeQL model does not resolve MapValue for wildcard-typed maps, the taint propagation will silently fail, causing missed flows through URI template expansion.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| data: | ||
| - ["io.micronaut.http.client", "BlockingHttpClient", True, "exchange", "(HttpRequest)", "", "Argument[0]", "request-forgery", "manual"] | ||
| - ["io.micronaut.http.client", "BlockingHttpClient", True, "exchange", "(HttpRequest,Argument)", "", "Argument[0]", "request-forgery", "manual"] | ||
| - ["io.micronaut.http.client", "BlockingHttpClient", True, "exchange", "(HttpRequest,Argument,Argument)", "", "Argument[0]", "request-forgery", "manual"] |
There was a problem hiding this comment.
Shipwright · HIGH
The HTTP client sink model marks only Argument[0] of exchange/retrieve as request-forgery.
Impact: The HTTP client sink model marks only Argument[0] of exchange/retrieve as request-forgery. For overloads taking a String URI, the URI is the request target and should be modeled, but the sink kind request-forgery on a String argument may not match the expected taint source for URL manipulation queries, potentially missing SSRF-like flows through user-controlled URI strings.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| --- | ||
| category: minorAnalysis | ||
| --- | ||
| * Added modeling for the Micronaut framework, including HTTP controllers, WebSocket endpoints, configuration injection, data access, security annotations, and HTTP client sinks. |
There was a problem hiding this comment.
Shipwright · LOW
The change note claims modeling for 'data access' and 'security annotations', but the added MicronautData.qll and MicronautSecurity.qll only define annotation/class recognition wit
Impact: The change note claims modeling for 'data access' and 'security annotations', but the added MicronautData.qll and MicronautSecurity.qll only define annotation/class recognition with no dataflow integration or query behavior. A maintainer reading the change note will expect functional modeling that does not exist yet.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
1 participant
This pull request adds CodeQL modeling for the Micronaut Java framework, covering HTTP controllers, WebSocket endpoints, configuration injection, security annotations, and relevant sources and sinks. The changes integrate Micronaut-specific classes and methods into the CodeQL dataflow architecture, enabling taint tracking and threat modeling for user input, configuration, and HTTP/WebSocket requests. Test cases are included to verify the new models.
Micronaut framework modeling:
MicronautController.qll, enabling detection and classification of user-controlled input sources.MicronautWebSocket.qll.@Valueand@Propertyannotations, modeling fields and parameters as local user input inMicronautConfig.qll.MicronautData.qll.@Securedannotations for classes and methods inMicronautSecurity.qll.Dataflow source and sink integration:
FlowSources.qllto include Micronaut HTTP input parameters, WebSocket parameters, configuration fields/parameters, and error handler parameters as sources, using the new overlays. [1] [2].model.ymlfiles, covering taint propagation and SSRF, response splitting, and URL redirection sinks. [1] [2] [3]Test coverage:
MicronautControllerTest.java.MicronautConfigTest.java.Source merge-base:
d045392042b2233c7cefa615c3cb5814a201c37bSource head:
16e9b22b333cdc4cef4244baa8d162da55fc22eb