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

Unreasonable Size Argument - OS_CreateSocketName static analysis warning #817

Closed
skliper opened this issue Feb 17, 2021 · 0 comments · Fixed by #818 or #830
Closed

Unreasonable Size Argument - OS_CreateSocketName static analysis warning #817

skliper opened this issue Feb 17, 2021 · 0 comments · Fixed by #818 or #830
Assignees
Labels
Milestone

Comments

@skliper
Copy link
Contributor

skliper commented Feb 17, 2021

Is your feature request related to a problem? Please describe.
Static analysis warns when using sizeof(sock->stream_name) in OS_strnlen check and later math OS_MAX_API_NAME - len passed to snprintf which out of context could then be a negative number (but isn't because OS_SocketAddrToString_Impl limits to OS_MAX_API_NAME, so this is a false positive):

if (OS_SocketAddrToString_Impl(sock->stream_name, OS_MAX_API_NAME, Addr) != OS_SUCCESS)
{
sock->stream_name[0] = 0;
}
if (OS_SocketAddrGetPort_Impl(&port, Addr) == OS_SUCCESS)
{
len = OS_strnlen(sock->stream_name, sizeof(sock->stream_name));
snprintf(&sock->stream_name[len], OS_MAX_API_NAME - len, ":%u", (unsigned int)port);
}
sock->stream_name[OS_MAX_API_NAME - 1] = 0;

Describe the solution you'd like
Truncating the port while fully adding the parent name or possibly even truncating both seems like it could be confusing. Just truncate at the end.

Describe alternatives you've considered
Could use OS_MAX_API_NAME to limit len in first check, but seems like overkill since the size is OS_MAX_PATH_LEN.

Additional context
Static analysis warning

Requester Info
Jacob Hageman - NASA/GSFC

@skliper skliper added this to the 6.0.0 milestone Feb 17, 2021
@skliper skliper self-assigned this Feb 17, 2021
skliper added a commit to skliper/osal that referenced this issue Feb 17, 2021
@astrogeco astrogeco added bug and removed enhancement labels Feb 24, 2021
astrogeco added a commit that referenced this issue Feb 24, 2021
Fix #817, Simplify name truncation in OS_CreateSocketName
jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants