Skip to content

C++: Rewrite cpp/cgi-xss to not use default taint tracking #14424

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

Merged
merged 2 commits into from
Oct 13, 2023

Conversation

jketema
Copy link
Contributor

@jketema jketema commented Oct 10, 2023

Also add a test that demonstrates that we need to look at inidrect expressions and not direct ones.

New attempt at this, now using indirect expressions. MRVA and DCA still don't show anything interesting.

@github-actions github-actions bot added the C++ label Oct 10, 2023
Also add a test that demonstrates that we need to look at inidrect expressions
and not direct ones.
@jketema jketema marked this pull request as ready for review October 11, 2023 08:37
@jketema jketema requested a review from a team as a code owner October 11, 2023 08:37
@jketema jketema added the no-change-note-required This PR does not need a change note label Oct 11, 2023
MathiasVP
MathiasVP previously approved these changes Oct 11, 2023
Copy link
Contributor

@MathiasVP MathiasVP left a comment

Choose a reason for hiding this comment

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

LGTM!

@jketema
Copy link
Contributor Author

jketema commented Oct 11, 2023

Before we merge this, let me address the double issue that we spotted, and let me investigate the result differences we saw on two projects.

@jketema
Copy link
Contributor Author

jketema commented Oct 13, 2023

Fixed the barrier. MRVA looks good not changes compared to before the rewrite on the projects that we had identified as either (a) being in the MRVA top 1000 and having a relevant source, or (b) a project that we know had alerts before the rewrite.

Will rerun DCA.

Copy link
Contributor

@MathiasVP MathiasVP left a comment

Choose a reason for hiding this comment

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

LGTM if DCA is happy!

@jketema
Copy link
Contributor Author

jketema commented Oct 13, 2023

DCA looks good.

@jketema jketema merged commit d56a9f0 into github:main Oct 13, 2023
@jketema jketema deleted the rewrite-cgi-xss branch October 13, 2023 15:57
@github github deleted a comment from Horvathbarbara1219 Oct 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C++ no-change-note-required This PR does not need a change note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants