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

Remove remaining stray uses of sprintf() #2172

Open
2 tasks done
thnkslprpt opened this issue Oct 17, 2022 · 1 comment · May be fixed by #2173
Open
2 tasks done

Remove remaining stray uses of sprintf() #2172

thnkslprpt opened this issue Oct 17, 2022 · 1 comment · May be fixed by #2173

Comments

@thnkslprpt
Copy link
Contributor

thnkslprpt commented Oct 17, 2022

Checklist

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
A few stray uses of sprintf() remain in cFE (mostly in the test code, but not entirely).
The simple cases will be converted to snprintf() to enforce a maximum size for the buffer being written into.
A couple of cases have their own issues open already with further changes being considered, so I will probably leave those to be addressed there (#1465 and #1511)

Expected behavior
No use of sprintf() in the code base (including test code).

Code snips

sprintf(FullCDSName, "%s.%s", AppName, CDSName);

sprintf(TblName, "%d", i);

Reporter Info
Avi Weiss @thnkslprpt

Previous discussion opening this issue:
I concur that snprintf has some caveats/dangers - particularly with respect to the return code and handling that properly - whereas sprintf() is a buffer overflow exploit in almost all use-cases, and should rightfully be banned.
Originally posted by @jphickey in #824 (comment)

There are still a few stray uses of sprintf() across cFS. Should these be removed?

@skliper
Copy link
Contributor

skliper commented Oct 17, 2022

Yeah, that would be a good update from my point of view. Most of it's in tests which might not be a high priority, but there's a handful of flight code use still.

@thnkslprpt thnkslprpt changed the title Remove the few remaining stray uses of sprintf()? Remove remaining stray uses of sprintf()? Oct 18, 2022
@thnkslprpt thnkslprpt changed the title Remove remaining stray uses of sprintf()? Remove remaining stray uses of sprintf() Oct 18, 2022
thnkslprpt added a commit to thnkslprpt/cFE that referenced this issue Oct 18, 2022
thnkslprpt added a commit to thnkslprpt/cFE that referenced this issue Oct 18, 2022
@thnkslprpt thnkslprpt linked a pull request Oct 18, 2022 that will close this issue
2 tasks
thnkslprpt added a commit to thnkslprpt/cFE that referenced this issue Oct 21, 2022
thnkslprpt added a commit to thnkslprpt/cFE that referenced this issue Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants