Skip to content
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

Go: Introduce Threat Modeling #16697

Merged

Conversation

egregius313
Copy link
Contributor

@egregius313 egregius313 commented Jun 7, 2024

Initial implementation of threat modeling in Go. Based on #15359 which introduced the threat modeling in C#.

The terms in the getSourceType implementations might not be the best for each individual remote source.

Note

The Go library currently does not appear to have any local sources modeled, so this currently only handles remote flow sources.

Important

This does not change the queries and libraries to use ThreatModelFlowSource. This merely introduces the class and a few minor changes to the class hierarchy to make later changes easier.
Changing queries to use ThreatModelFlowSource instead of RemoteFlowSource is tracked in #16709.

@egregius313 egregius313 force-pushed the egregius313/go/dataflow/threat-modeling branch 3 times, most recently from 491bd7c to 413fa9a Compare June 7, 2024 06:51
@egregius313 egregius313 marked this pull request as ready for review June 7, 2024 06:51
@egregius313 egregius313 requested a review from a team as a code owner June 7, 2024 06:51
@egregius313 egregius313 force-pushed the egregius313/go/dataflow/threat-modeling branch from 413fa9a to 03aa05c Compare June 7, 2024 14:43
owen-mc
owen-mc previously approved these changes Jun 11, 2024
Copy link
Contributor

@owen-mc owen-mc left a comment

Choose a reason for hiding this comment

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

Looks great! One question: what is the string from getSourceType() ever used for? There are some cases where it could be more precise/consistent, but then I didn't actually see its value being used for anything so I wasn't sure if that was important.

@egregius313
Copy link
Contributor Author

One question: what is the string from getSourceType() ever used for? There are some cases where it could be more precise/consistent, but then I didn't actually see its value being used for anything so I wasn't sure if that was important.

This might not be necessary for Go. In the C# library, this is used for some of the queries which give some extra information about the source in the alert message. For example:

string getSourceType(DataFlow::Node node) { result = node.(SourceNode).getSourceType() }
from SqlInjection::PathNode source, SqlInjection::PathNode sink
where SqlInjection::flowPath(source, sink)
select sink.getNode(), source, sink, "This query depends on $@.", source,
("this " + getSourceType(source.getNode()))

I can remove the getSourceType definitions if you would like.

@owen-mc
Copy link
Contributor

owen-mc commented Jun 12, 2024

I ran a poll amongst our colleagues and they unanimously voted for not introducing getSourceType. If you remove it in a separate commit then that commit can always be reverted if someone wants it in future. It sounds like they want to deprecate it and eventually remove it for java as well.

@egregius313
Copy link
Contributor Author

@owen-mc sorry for the delay, but I have added the commit to remove getSourceType

@egregius313 egregius313 requested a review from owen-mc June 17, 2024 15:07
@egregius313 egregius313 merged commit 8997f2c into github:main Jun 18, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants