Skip to content

C#: Add queries to check untrusted data flow to external APIs - #4678

Merged
tamasvajk merged 4 commits into
github:mainfrom
tamasvajk:feature/external-api-untrusted-data
Dec 4, 2020
Merged

C#: Add queries to check untrusted data flow to external APIs#4678
tamasvajk merged 4 commits into
github:mainfrom
tamasvajk:feature/external-api-untrusted-data

Conversation

@tamasvajk

@tamasvajk tamasvajk commented Nov 17, 2020

Copy link
Copy Markdown
Contributor

This is a port of the Java matching checks.

The main differences are that

  • we're not only reporting Methods but Callables,
  • SafeExternalAPICallable adds only a super minimal safe API list for the time being.

@tamasvajk

Copy link
Copy Markdown
Contributor Author

Differences job

@hvitved hvitved left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A few minor comments, otherwise LGTM. Great to have this query in the toolbox.

Comment thread csharp/ql/src/Security Features/CWE-020/ExternalAPIsUsedWithUntrustedData.ql Outdated
Comment thread csharp/ql/src/semmle/code/csharp/security/dataflow/ExternalAPIs.qll Outdated
Comment thread csharp/ql/src/semmle/code/csharp/security/dataflow/ExternalAPIs.qll Outdated
Comment thread csharp/ql/src/semmle/code/csharp/security/dataflow/ExternalAPIs.qll Outdated
Comment thread csharp/ql/src/semmle/code/csharp/security/dataflow/ExternalAPIs.qll
@@ -1,2 +1,3 @@
| System.Collections.Specialized.NameValueCollection.get_Item(string) [qualifier] | 1 | 1 |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Interesting; we should model this (like we model dictionaries). (Not on this PR.)

hvitved
hvitved previously approved these changes Dec 4, 2020
@tamasvajk
tamasvajk force-pushed the feature/external-api-untrusted-data branch from 1eff2a3 to aa3ae0f Compare December 4, 2020 12:28
@tamasvajk

Copy link
Copy Markdown
Contributor Author

I rebased and pushed an extra commit to fix the CI check failure.

@tamasvajk
tamasvajk merged commit 4226467 into github:main Dec 4, 2020
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.

2 participants