Skip to content

Commit

Permalink
Fix #63, clean up strncpy calls
Browse files Browse the repository at this point in the history
Call "strncpy" with the size parameter indicating the size of the
destination buffer, rather than the input string length.

A buffer overflow was avoided due to a length check already in the code,
but calling the function properly should avoid a warning.
  • Loading branch information
jphickey committed Nov 14, 2022
1 parent 23b792d commit 8517134
Showing 1 changed file with 18 additions and 17 deletions.
35 changes: 18 additions & 17 deletions fsw/src/fm_child.c
Original file line number Diff line number Diff line change
Expand Up @@ -1221,17 +1221,16 @@ void FM_ChildDirListPktCmd(const FM_ChildQueueEntry_t *CmdArgs)
EntryLength = strlen(OS_DIRENTRY_NAME(DirEntry));

/* Verify combined directory plus filename length */
if ((PathLength + EntryLength) < OS_MAX_PATH_LEN)
if ((PathLength + EntryLength) < sizeof(LogicalName))
{
/* Add filename to directory listing telemetry packet */
strncpy(ListEntry->EntryName, OS_DIRENTRY_NAME(DirEntry), EntryLength);
ListEntry->EntryName[EntryLength] = '\0';
strncpy(ListEntry->EntryName, OS_DIRENTRY_NAME(DirEntry), sizeof(ListEntry->EntryName) - 1);
ListEntry->EntryName[sizeof(ListEntry->EntryName) - 1] = '\0';

/* Build filename - Directory already has path separator */
strncpy(LogicalName, CmdArgs->Source2, PathLength);
LogicalName[PathLength] = '\0';

strncat(LogicalName, OS_DIRENTRY_NAME(DirEntry), EntryLength);
memcpy(LogicalName, CmdArgs->Source2, PathLength);
memcpy(&LogicalName[PathLength], OS_DIRENTRY_NAME(DirEntry), EntryLength);
LogicalName[PathLength + EntryLength] = '\0';

FM_ChildSleepStat(LogicalName, ListEntry, &FilesTillSleep, CmdArgs->GetSizeTimeMode);

Expand Down Expand Up @@ -1437,20 +1436,22 @@ void FM_ChildDirListFileLoop(osal_id_t DirId, osal_id_t FileHandle, const char *
* DirListData.EntryName and TempName are both OS_MAX_PATH_LEN, DirEntry name is OS_MAX_FILE_NAME,
* so limiting test is PathLength and EntryLength together
*/
if ((PathLength + EntryLength) < OS_MAX_PATH_LEN)
if ((PathLength + EntryLength) < sizeof(TempName))
{
/* Build qualified directory entry name */
strncpy(TempName, DirWithSep, PathLength);
TempName[PathLength] = '\0';

strncat(TempName, OS_DIRENTRY_NAME(DirEntry), (OS_MAX_PATH_LEN - PathLength));

/* Populate directory list file entry */
memcpy(TempName, DirWithSep, PathLength);
memcpy(&TempName[PathLength], OS_DIRENTRY_NAME(DirEntry), EntryLength);
TempName[PathLength + EntryLength] = '\0';

/*
* Populate directory list file entry -
* Note this is guaranteed to be null terminated due to the memset()
* this will leave at least one null char after the string.
*/
memset(&DirListData, 0, sizeof(DirListData));
strncpy(DirListData.EntryName, OS_DIRENTRY_NAME(DirEntry), EntryLength);
DirListData.EntryName[EntryLength] = '\0';
strncpy(DirListData.EntryName, OS_DIRENTRY_NAME(DirEntry), sizeof(DirListData.EntryName) - 1);

FM_ChildSleepStat(TempName, (FM_DirListEntry_t *)&DirListData, &FilesTillSleep, getSizeTimeMode);
FM_ChildSleepStat(TempName, &DirListData, &FilesTillSleep, getSizeTimeMode);

/* Write directory list file entry to output file */
BytesWritten = OS_write(FileHandle, &DirListData, WriteLength);
Expand Down

0 comments on commit 8517134

Please sign in to comment.