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.

Sign up for GitHub

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

Jump to bottom

C#: Add missing CWE tags #16461

Merged
merged 2 commits into from May 14, 2024
Merged

C#: Add missing CWE tags #16461

merged 2 commits into from May 14, 2024

Conversation

joefarebrother
Copy link
Contributor

Adds cwe-1173 to cs/ambiguous-client-variable and cs/ambiguous-server-variable.

@joefarebrother joefarebrother requested a review from a team as a code owner May 9, 2024 09:36
@github-actions github-actions bot added the C# label May 9, 2024
@joefarebrother joefarebrother added no-change-note-required This PR does not need a change note and removed C# labels May 9, 2024
@joefarebrother joefarebrother mentioned this pull request May 9, 2024
Closed
michaelnebel
michaelnebel reviewed May 13, 2024
View reviewed changes
Copy link
Contributor

@michaelnebel michaelnebel left a comment

Choose a reason for hiding this comment

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

This looks good to me, but maybe we should also ask someone with a bit more CWE domain knowledge.
@atorralba : Do you think this is a reasonable CWE tag to add to these queries (we just need to add at least one)?

@atorralba
Copy link
Contributor

This looks good to me, but maybe we should also ask someone with a bit more CWE domain knowledge. @atorralba : Do you think this is a reasonable CWE tag to add to these queries (we just need to add at least one)?

CWE-1173 is a bit generic in my opinion and doesn't particularly apply to this vulnerability pattern.

After some time reviewing CWEs, I think CWE-348: Use of less trusted source is a better fit. The examples are a bit fixated on bypassing IP controls with the X-Forwarded-For HTTP header, but the underlying principle is the same IMHO.

@github-actions github-actions bot added the C# label May 13, 2024
atorralba
atorralba approved these changes May 14, 2024
View reviewed changes
Copy link
Contributor

@atorralba atorralba left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@michaelnebel
Copy link
Contributor

Thank you @atorralba !

michaelnebel
michaelnebel approved these changes May 14, 2024
View reviewed changes
Copy link
Contributor

@michaelnebel michaelnebel left a comment

Choose a reason for hiding this comment

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

👍

@joefarebrother joefarebrother merged commit a62ce4c into github:main May 14, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michaelnebel michaelnebel michaelnebel approved these changes

@atorralba atorralba atorralba approved these changes

Assignees
No one assigned
Labels
C# no-change-note-required This PR does not need a change note
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@joefarebrother @atorralba @michaelnebel