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

CF method CF_CFDP_SendNak ph value can never be NULL #24

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

CF method CF_CFDP_SendNak ph value can never be NULL #24

jphickey opened this issue Nov 23, 2021 · 2 comments · Fixed by #137
Milestone

Comments

@jphickey
Copy link
Contributor

This issue was imported from the GSFC issue tracking system

Imported from: [GSFCCFS-1689] CF method CF_CFDP_SendNak ph value can never be NULL
Originally submitted by: Gibson, Alan S. (GSFC-5870) on Fri Jul 30 17:27:32 2021

Original Description:
The CF_CFDP_SendNak function receives it's header from &((pdu_s_msg_t*)CF_AppData.engine.out.msg)->ph but even when CF_AppData.engine.out.msg == NULL, ph is 0x10.

The check if ph is NULL (if (!ph)) will never be able to fire.

The check should be changed to see if CF_AppData.engine.out.msg is NULL instead; this will ensure that if/when it is NULL a ph value of 0x10 will not be used.

Note: This is probably not possible in the field, a "nack" should never be able to be returned without msg being populated, but in that case there is no reason to verify ph is not NULL.

@jphickey
Copy link
Contributor Author

Imported comment by internal user on Tue Nov 16 14:26:56 2021

I also noted this when reviewing the code, this gets the base pointer from a global variable which can be NULL in some circumstances. Therefore for safety this should always check that the base pointer is non-NULL before doing the pointer offset arithmetic here.

@jphickey
Copy link
Contributor Author

jphickey commented Jan 6, 2022

This is fixed in #137, the code does not directly access an encoded header any more.

@skliper skliper added this to the Draco milestone Jan 7, 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.

2 participants