-
|
I have this go function: func (s *Server) ListTriggers(ctx context.Context, q *notification.TriggersListRequest) (*notification.TriggerList, error) {
[...]
}You can find it here: https://github.com/argoproj/argo-cd/blob/v2.13.3/server/notification/notification.go#L25 I want to add the extensions:
- addsTo:
pack: codeql/go-all
extensible: sourceModel
data:
- ["github.com/argoproj/argo-cd/v2/server/notification", "", True, "ListTriggers", "", "", "Argument[1]", "remote", "manual" ]
- addsTo:
pack: codeql/go-all
extensible: sinkModel
data: []
- addsTo:
pack: codeql/go-all
extensible: summaryModel
data: []
I'm running the following query to check if I have additional entries in the output with the model: import semmle.go.security.FlowSources
from RemoteFlowSource::Range source
where not source.getFile().getRelativePath().matches("%/test/%")
select source, ""But I get the same number of result with and without my model pack. So is my model pack correct ? How can I test if it's really adding sources ? Here is my command line: $ codeql database analyze /../argo-cd-v2.13.3 --format=sarif-latest --output=/../sarif-remote-flow-sources.sarif --additional-packs /.../argocd-cd-ext/ --additional-packs /../argocd-cd/ --model-packs synacktiv/argocd-cd-ext synacktiv/argocd-cd:query.ql --rerun |
Beta Was this translation helpful? Give feedback.
Replies: 3 comments 23 replies
-
|
Thank you for your detailed question. Looking at your Go model, it is missing the receiver type in the Type column. Try this instead. If that does not work then it may be a model pack issue. |
Beta Was this translation helpful? Give feedback.
-
|
I don't think we use I'm not sure what you mean about there being no code. If there are no calls to a function then we may not be able to model it using a yml file, but we might be able to do it in CodeQL. Can you give me an example of something you'd want CodeQL to find as a source of taint? |
Beta Was this translation helpful? Give feedback.
-
|
Ok so now I have an additional result: Yes in my case I would like to restrict it to the case when the receiver is a |
Beta Was this translation helpful? Give feedback.
-
|
I want to taint my parameter func (s *Server) ListTriggers(ctx context.Context, q *notification.TriggersListRequest) (*notification.TriggerList, error) {
foo.callMethodA(q)
}
func ... callMethodA(q *notification.TriggersListRequest){
bar.callMethodB(q)
}
func ... callMethodB(q *notification.TriggersListRequest){
ba.sinkMethod(q)
}In my case I know that I can control I want to do this with the model feature to avoid adding |
Beta Was this translation helpful? Give feedback.
-
|
Thanks for explaining your use case. This does indeed seem to be what "Parameter", which @hvitved mentioned earlier, does. I haven't used it before, and it isn't currently in use in the Go library, but it is possible that it currrently works, or that minimal changes are needed to make it work. I will look into it. In the meantime, have you tried the following model? |
Beta Was this translation helpful? Give feedback.
-
|
Thanks for the information, yes following the comment of @hvitved I tried this, but it does not return additional sources. When you say minimal changes are needed, do you have an idea of how to do this ? I know nothing about the codeql In the |
Beta Was this translation helpful? Give feedback.
-
|
Thanks for confirming that it doesn't work for you. I found the same with some internal tests. To be clear, I do not expect you to get "Parameter" working. It is something that we should fix. If it is easy to fix then I will try to do it soon. Otherwise it will have to be prioritised alongside other work. I believe it shouldn't be too much work as most of the logic is in a shared library, and that clearly works for other languages. Once we have found a fix and a PR is merged, it will work with the next release of CodeQL. Releases are made roughly every 2 weeks. Thank you for highlighting this shortcoming. |
Beta Was this translation helpful? Give feedback.
-
|
Alright, thank you very much! Can I leave this unresolved and wait for the feature to be implemented, so you can update me once it's done? Otherwise, I have no problem closing it. Anyway, thank you again for taking the time to help me and for providing all the detailed information. This isn’t the first time I’ve posted a message in the CodeQL discussions, and every time, people take the time to explain things clearly and thoroughly, which I truly appreciate. |
Beta Was this translation helpful? Give feedback.
-
|
I believe I've found the problem and this PR should fix it. Yes, feel free to leave this discussion open. |
Beta Was this translation helpful? Give feedback.
-
|
Hi @owen-mc I have an additional issue with the model pack. I've applied your patch and I'm trying to use my model pack with the But If I copy the same query in my custom pack I got some results: What's the difference ? I've also tried to use the syntax |
Beta Was this translation helpful? Give feedback.
-
|
Sorry, I'm also no expert on model packs, but can you try to take a look at the logs in both cases, to see if they both actually load the model pack correctly? |
Beta Was this translation helpful? Give feedback.
-
|
Yes they are all loaded in both cases: |
Beta Was this translation helpful? Give feedback.
-
|
I'm suspecting that the patch you applied from @owen-mc is perhaps not picked up when you run the query |
Beta Was this translation helpful? Give feedback.
-
|
I defer to @hvitved as I am not very familiar with model packs. |
Beta Was this translation helpful? Give feedback.
Thanks for explaining your use case. This does indeed seem to be what "Parameter", which @hvitved mentioned earlier, does. I haven't used it before, and it isn't currently in use in the Go library, but it is possible that it currrently works, or that minimal changes are needed to make it work. I will look into it. In the meantime, have you tried the following model?