Skip to content

Use StrcpyFunction in cpp/bad-strncpy-size #9998

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 5 commits into from
Aug 10, 2022

Conversation

d10c
Copy link
Contributor

@d10c d10c commented Aug 9, 2022

This PR:

  • Uses the StrcpyFunction class in the StrncpyFlippedArgs query instead of an ad-hoc predicate for finding strcpy-like functions.
  • Tests this by adding one previously unsupported strcpy-like function (wcsxfrm_l) to StrncpyFlippedArgs's test.cpp.

@d10c d10c requested a review from a team as a code owner August 9, 2022 17:10
@github-actions github-actions bot added the C++ label Aug 9, 2022
@d10c d10c marked this pull request as draft August 9, 2022 17:51
Copy link
Contributor

@geoffw0 geoffw0 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 great.

I would include a short change note, as the query now finds more results.

wchar_t buf2[20];
const wchar_t *str = L"01234567890123456789";

wcsxfrm_l(buf1, str, sizeof(buf1), nullptr); // BAD (but not a StrncpyFlippedArgs bug)
Copy link
Contributor

Choose a reason for hiding this comment

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

(but not a StrncpyFlippedArgs bug)

I'm not sure what you're saying here, the query appears to have flagged this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, good point... test9() is a copy of test2() pretty much, including the comments, and has the same behaviour. Namely, that line is flagged because (I'm guessing) it falls under the "Fixed sized case", which is not exactly a case of "flipped" args. At least, that's how I interpreted the original comment.

d10c added 4 commits August 10, 2022 13:42
Added wcsxfrm_l, which is not currently caught by the query,
meaning that in this case a successful
test implies missing functionality.
As a result, the query gets access to more types of strncpy-like
functions, as demonstrated by test.cpp, which now "fails" (i.e. works) for the new test
cases instroduced
in the previous commit.
Add output lines for the newly implemented test case, test.cpp/test9().
@d10c d10c force-pushed the use-strcpyfunction-in-bad-strncpy-size branch from 265e3d8 to 05f4f98 Compare August 10, 2022 11:42
@d10c d10c marked this pull request as ready for review August 10, 2022 11:43
geoffw0
geoffw0 previously approved these changes Aug 10, 2022
Copy link
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@d10c d10c merged commit cce39fb into github:main Aug 10, 2022
@d10c d10c deleted the use-strcpyfunction-in-bad-strncpy-size branch September 22, 2022 14:08
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