-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[Golang] Additional Taint Step hidden in PathGraph Visualisation #20596
-
Hello,
I added a taint step by extending TaintTracking::AdditionalTaintStep in golang. Now codeql builds the path with this step correctly, but I don't see the step itself in the alerts section, version 2.21.0.
Here is a simplified code snippet for my query, which works like a charm:
// url comes already tainted
func GetImageInfo(url string) (int32, int32, error) {
...
req, err := http.NewRequest(http.MethodGet, url, nil) // my taint step: taint from url to the req variable
req = req.WithContext(extmon.MethodNameToCtx(req.Context(), "Get"))
var resp *http.Response
resp, err = transport.RoundTrip(req) // RoundTrip() is sink
...
}
The problem is, when I click through the steps in the alerts section, the only two steps for this function I see are:
func GetImageWH(url string) (int32, int32, error) { - definition of url
resp, err = transport.RoundTrip(req) - req
- How to make my customised taint step visible in the path?
- And when it becomes visible, I'd like to name the taint step as
NewRequestStepin the graph visualisation, instead ofdefinition of url. How can I customise this if possible?
I need to explicitly reflect in the query results that my taint step contributed to the flow, for my internal SAST metrics/statistics.
Thanks for taking time with this!
Beta Was this translation helpful? Give feedback.
All reactions
You could try in your data-flow configuration specifying predicate neverSkip(Node node) -- by default, the edges relation that populates the user-facing graph will skip nodes unless they are join points (have multiple predecessors) or are interprocedural edges.
The names printed in the path explanation refer to nodes, not edges, but you could try something like class CustomNamedNode extends DataFlow::Node { ... CustomNamedNode() { ... characterise your node ... } ... override string toString() { ... stringify your node ... }, and ensure your custom node definition is in scope in the context of whatever query produces your path explanation.
Replies: 2 comments 5 replies
-
You could try in your data-flow configuration specifying predicate neverSkip(Node node) -- by default, the edges relation that populates the user-facing graph will skip nodes unless they are join points (have multiple predecessors) or are interprocedural edges.
The names printed in the path explanation refer to nodes, not edges, but you could try something like class CustomNamedNode extends DataFlow::Node { ... CustomNamedNode() { ... characterise your node ... } ... override string toString() { ... stringify your node ... }, and ensure your custom node definition is in scope in the context of whatever query produces your path explanation.
Beta Was this translation helpful? Give feedback.
All reactions
-
Thank you so much! That helps a lot.
Beta Was this translation helpful? Give feedback.
All reactions
-
@smowton Hello,
Now I'm trying to do the same for Csharp, but Csharp DataFlow::Node has the final annotation so I can't override it:
final string toString() { result = this.(NodeImpl).toStringImpl() }.
Is there any way to solve it out?
Beta Was this translation helpful? Give feedback.
All reactions
-
Hmm, well you could wrap the types involved in the PathGraph's nodes, edges, subpaths relations? Remove import SomeFlow::PathGraph and instead do something like...
string customToString(PathNode n) { ... your custom stringification here ... } newtype TMyPathNode = TWrapPathNode(PathNode pn) class MyPathNode extends TMyPathNode { PathNode pn; MyPathNode() { this = TWrapPathNode(pn) } string toString() { result = customToString(pn) or not exists(customToString(pn)) and result = pn.toString() } PathNode getPathNode() { result = pn } } query predicate nodes(MyPathNode mpn) { SomeFlow::PathGraph::nodes(mpn.getPathNode()) } query predicate edges(MyPathNode mpn1, MyPathNode mpn2) { SomeFlow::PathGraph::edges(mpn1.getPathNode(), mpn2.getPathNode()) } query predicate subpaths(...) // similar
Beta Was this translation helpful? Give feedback.
All reactions
-
We already make sure that we never skip over steps coming from function models for any configuration. We could do additional taint steps too.
There are edge labels for some edges, e.g. those coming from function models or from additional flow steps specified in the flow configuration. For steps coming from TaintTracking::AdditionalTaintStep the label is "AdditionalTaintStep". We could make that customizable when you define the additional taint step.
Would either of these be helpful, @KseniiaSmirn0va ? Or has @smowton's suggestions already solved your problem?
Beta Was this translation helpful? Give feedback.
All reactions
-
Thank you, @owen-mc, for the suggestion! Did you mean a new patch is coming out for the feature I need?
Actually, I've solved out the matter with @smowton's suggestion
Beta Was this translation helpful? Give feedback.
All reactions
-
Glad to hear your problem is solved. In that case I will not implement the more general solution that I suggested.
Beta Was this translation helpful? Give feedback.