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

CF_TraverseHistory string buffer handling #40

Closed
jphickey opened this issue Nov 23, 2021 · 2 comments · Fixed by #174
Closed

CF_TraverseHistory string buffer handling #40

jphickey opened this issue Nov 23, 2021 · 2 comments · Fixed by #174
Assignees
@jphickey
Labels
draco-rc1
Milestone
Draco

Comments

@jphickey
Copy link
Contributor

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1776] CF_TraverseHistory string buffer handling
Originally submitted by: Hickey, Joseph P. (GSFC-582.0)[VANTAGE SYSTEMS INC] on Tue Nov 16 14:22:27 2021

Original Description:
This issue was originally reported by IV&V, creating Jira issue to track its disposition and resolution.

Function CF_TraverseHistory() in cf_utils.c in the CF App source code, writes some text to the buffer 'linebuf' on line 70. This buffer, however, is overwritten on line 74 before it is written to the file descriptor on line 75. It is questionable whether this was the desired behavior.
@jphickey
Copy link
Contributor Author

Imported comment by internal user on Tue Nov 16 14:24:57 2021

The report from IV&V is valid in that this overwrites some potentially desired parts of the human-readable output, but misses a potentially more significant issue in that the return value from snprintf in this block is then passed to the CF_WrappedWrite() function directly. However the return value from snprintf is not necessarily the length of the string. Using it as such may cause a read beyond the end of the valid buffer.

@jphickey jphickey self-assigned this Jan 6, 2022
@jphickey
Copy link
Contributor Author

jphickey commented Jan 6, 2022

Definitely need to fix this one, since it is a potential read beyond buffer bounds situation.

jphickey added a commit to jphickey/CF that referenced this issue Jan 12, 2022
@jphickey
Fix nasa#40, update traverse history/write file
1fd43a3 
- rename the function to better indicate what it does
- do not discard the part of the output that has EID/TSN/CC information
- do not pass the return value of snprintf directly to write(), use strlen()
- simplify the code
jphickey added a commit to jphickey/CF that referenced this issue Jan 13, 2022
@jphickey
Fix nasa#40, update traverse history/write file
9e69d62 
- rename the function to better indicate what it does
- do not discard the part of the output that has EID/TSN/CC information
- do not pass the return value of snprintf directly to write(), use strlen()
- simplify the code
jphickey added a commit to jphickey/CF that referenced this issue Jan 13, 2022
@jphickey
Fix nasa#40, update traverse history/write file
045c739 
- rename the function to better indicate what it does
- do not discard the part of the output that has EID/TSN/CC information
- do not pass the return value of snprintf directly to write(), use strlen()
- simplify the code
@jphickey jphickey mentioned this issue Jan 13, 2022
Merged
astrogeco added a commit that referenced this issue Jan 18, 2022
@astrogeco
Merge pull request #174 from jphickey/fix-40-writefile
fe02332 
Fix #40, update traverse history/write file
@skliper skliper added this to the Draco milestone Mar 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jphickey jphickey

Labels
draco-rc1
Projects
None yet
Milestone
Draco
Development

Successfully merging a pull request may close this issue.

Fix #40, update traverse history/write file jphickey/CF
2 participants
@jphickey @skliper