Skip to content

Commit

Permalink
Merge pull request #1140 from zanzaben/fix546_API_argument_validation
Browse files Browse the repository at this point in the history
Fix #546 #547, api argument validation
  • Loading branch information
astrogeco committed Mar 2, 2021
2 parents 454f035 + 260570d commit b50ae63
Show file tree
Hide file tree
Showing 9 changed files with 208 additions and 34 deletions.
34 changes: 28 additions & 6 deletions fsw/cfe-core/src/es/cfe_es_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -1590,6 +1590,11 @@ int32 CFE_ES_WriteToSysLog(const char *SpecStringPtr, ...)
int32 ReturnCode;
va_list ArgPtr;

if (SpecStringPtr == NULL)
{
return CFE_ES_BAD_ARGUMENT;
}

va_start(ArgPtr, SpecStringPtr);
CFE_ES_SysLog_vsnprintf(TmpString, sizeof(TmpString), SpecStringPtr, ArgPtr);
va_end(ArgPtr);
Expand Down Expand Up @@ -1659,6 +1664,11 @@ uint32 CFE_ES_CalculateCRC(const void *DataPtr, size_t DataLength, uint32 InputC

};

if (DataPtr == NULL || DataLength == 0)
{
return InputCRC;
}

switch(TypeCRC)
{
case CFE_MISSION_ES_CRC_32:
Expand Down Expand Up @@ -1709,12 +1719,17 @@ int32 CFE_ES_RegisterCDS(CFE_ES_CDSHandle_t *CDSHandlePtr, size_t BlockSize, con
char AppName[OS_MAX_API_NAME] = {"UNKNOWN"};
char CDSName[CFE_MISSION_ES_CDS_MAX_FULL_NAME_LEN] = {""};

/* Initialize output to safe value, in case this fails */
*CDSHandlePtr = CFE_ES_CDS_BAD_HANDLE;

/* Check to make sure calling application is legit */
Status = CFE_ES_GetAppID(&ThisAppId);

if (CDSHandlePtr == NULL || Name == NULL){
CFE_ES_WriteToSysLog("CFE_ES_RegisterCDS:-Failed invalid arguments\n");
return CFE_ES_BAD_ARGUMENT;
}

/* Initialize output to safe value, in case this fails */
*CDSHandlePtr = CFE_ES_CDS_BAD_HANDLE;

if ( Status != CFE_SUCCESS ) /* Application ID was invalid */
{
CFE_ES_WriteToSysLog("CFE_CDS:Register-Bad AppId context\n");
Expand All @@ -1726,9 +1741,6 @@ int32 CFE_ES_RegisterCDS(CFE_ES_CDSHandle_t *CDSHandlePtr, size_t BlockSize, con
}
else
{
/* Assume we can't make a CDS and return a bad handle for now */
*CDSHandlePtr = CFE_ES_CDS_BAD_HANDLE;

/* Make sure specified CDS name is not too long or too short */
NameLen = strlen(Name);
if ((NameLen > CFE_MISSION_ES_CDS_MAX_NAME_LENGTH) || (NameLen == 0))
Expand Down Expand Up @@ -1863,6 +1875,11 @@ CFE_Status_t CFE_ES_GetCDSBlockName(char *BlockName, CFE_ES_CDSHandle_t BlockId,
*/
int32 CFE_ES_CopyToCDS(CFE_ES_CDSHandle_t Handle, void *DataToCopy)
{
if (DataToCopy == NULL)
{
return CFE_ES_BAD_ARGUMENT;
}

return CFE_ES_CDSBlockWrite(Handle, DataToCopy);
} /* End of CFE_ES_CopyToCDS() */

Expand All @@ -1874,6 +1891,11 @@ int32 CFE_ES_CopyToCDS(CFE_ES_CDSHandle_t Handle, void *DataToCopy)
*/
int32 CFE_ES_RestoreFromCDS(void *RestoreToMemory, CFE_ES_CDSHandle_t Handle)
{
if (RestoreToMemory == NULL)
{
return CFE_ES_BAD_ARGUMENT;
}

return CFE_ES_CDSBlockRead(RestoreToMemory, Handle);
} /* End of CFE_ES_RestoreFromCDS() */

Expand Down
20 changes: 20 additions & 0 deletions fsw/cfe-core/src/es/cfe_es_mempool.c
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,11 @@ int32 CFE_ES_GetPoolBuf(CFE_ES_MemPoolBuf_t *BufPtr,
CFE_ES_MemPoolRecord_t *PoolRecPtr;
size_t DataOffset;

if (BufPtr == NULL)
{
return CFE_ES_BAD_ARGUMENT;
}

PoolRecPtr = CFE_ES_LocateMemPoolRecordByID(Handle);

/* basic sanity check */
Expand Down Expand Up @@ -473,6 +478,11 @@ int32 CFE_ES_GetPoolBufInfo(CFE_ES_MemHandle_t Handle,
size_t DataOffset;
size_t DataSize;

if (BufPtr == NULL)
{
return CFE_ES_BAD_ARGUMENT;
}

PoolRecPtr = CFE_ES_LocateMemPoolRecordByID(Handle);

/* basic sanity check */
Expand Down Expand Up @@ -527,6 +537,11 @@ int32 CFE_ES_PutPoolBuf(CFE_ES_MemHandle_t Handle,
size_t DataOffset;
int32 Status;

if (BufPtr == NULL)
{
return CFE_ES_BAD_ARGUMENT;
}

PoolRecPtr = CFE_ES_LocateMemPoolRecordByID(Handle);

/* basic sanity check */
Expand Down Expand Up @@ -605,6 +620,11 @@ int32 CFE_ES_GetMemPoolStats(CFE_ES_MemPoolStats_t *BufPtr,
uint16 NumBuckets;
uint16 Idx;

if (BufPtr == NULL)
{
return CFE_ES_BAD_ARGUMENT;
}

PoolRecPtr = CFE_ES_LocateMemPoolRecordByID(Handle);

/* basic sanity check */
Expand Down
12 changes: 12 additions & 0 deletions fsw/cfe-core/src/evs/cfe_evs.c
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,10 @@ int32 CFE_EVS_SendEvent (uint16 EventID, uint16 EventType, const char *Spec, ...
va_list Ptr;
EVS_AppData_t *AppDataPtr;

if(Spec == NULL){
return CFE_EVS_INVALID_PARAMETER;
}

/* Query and verify the caller's AppID */
Status = EVS_GetCurrentContext(&AppDataPtr, &AppID);
if (Status == CFE_SUCCESS)
Expand Down Expand Up @@ -190,6 +194,10 @@ int32 CFE_EVS_SendEventWithAppID (uint16 EventID, uint16 EventType, CFE_ES_AppId
va_list Ptr;
EVS_AppData_t *AppDataPtr;

if(Spec == NULL){
return CFE_EVS_INVALID_PARAMETER;
}

AppDataPtr = EVS_GetAppDataByID (AppID);
if (AppDataPtr == NULL)
{
Expand Down Expand Up @@ -225,6 +233,10 @@ int32 CFE_EVS_SendTimedEvent (CFE_TIME_SysTime_t Time, uint16 EventID, uint16 Ev
va_list Ptr;
EVS_AppData_t *AppDataPtr;

if(Spec == NULL){
return CFE_EVS_INVALID_PARAMETER;
}

/* Query and verify the caller's AppID */
Status = EVS_GetCurrentContext(&AppDataPtr, &AppID);
if (Status == CFE_SUCCESS)
Expand Down
23 changes: 20 additions & 3 deletions fsw/cfe-core/src/fs/cfe_fs_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ int32 CFE_FS_ReadHeader(CFE_FS_Header_t *Hdr, osal_id_t FileDes)
{
int32 Result;
int32 EndianCheck = 0x01020304;

if (Hdr == NULL)
{
return CFE_FS_BAD_ARGUMENT;
}

/*
** Ensure that we are at the start of the file...
Expand Down Expand Up @@ -81,9 +86,16 @@ int32 CFE_FS_ReadHeader(CFE_FS_Header_t *Hdr, osal_id_t FileDes)
*/
void CFE_FS_InitHeader(CFE_FS_Header_t *Hdr, const char *Description, uint32 SubType)
{
memset(Hdr, 0, sizeof(CFE_FS_Header_t));
strncpy((char *)Hdr->Description, Description, sizeof(Hdr->Description) - 1);
Hdr->SubType = SubType;
if(Hdr == NULL || Description == NULL)
{
CFE_ES_WriteToSysLog("CFE_FS:InitHeader-Failed invalid arguments\n");
}
else
{
memset(Hdr, 0, sizeof(CFE_FS_Header_t));
strncpy((char *)Hdr->Description, Description, sizeof(Hdr->Description) - 1);
Hdr->SubType = SubType;
}
}

/*
Expand All @@ -96,6 +108,11 @@ int32 CFE_FS_WriteHeader(osal_id_t FileDes, CFE_FS_Header_t *Hdr)
int32 EndianCheck = 0x01020304;
CFE_ES_AppId_t AppID;

if (Hdr == NULL)
{
return CFE_FS_BAD_ARGUMENT;
}

/*
** Ensure that we are at the start of the file...
*/
Expand Down
20 changes: 19 additions & 1 deletion fsw/cfe-core/src/inc/cfe_error.h
Original file line number Diff line number Diff line change
Expand Up @@ -1285,7 +1285,16 @@ typedef int32 CFE_Status_t;
** Error code indicating that the TBL file could not be
** opened by the OS.
*/
#define CFE_TBL_ERR_ACCESS ((CFE_Status_t)0xcc00002c)
#define CFE_TBL_ERR_ACCESS ((CFE_Status_t)0xcc00002c)

/**
* @brief Bad Argument
*
* A parameter given by a caller to a Table API did not pass
* validation checks.
*
*/
#define CFE_TBL_BAD_ARGUMENT ((CFE_Status_t)0xcc00002d)

/**
* @brief Not Implemented
Expand Down Expand Up @@ -1360,6 +1369,15 @@ typedef int32 CFE_Status_t;
*
*/
#define CFE_TIME_CALLBACK_NOT_REGISTERED ((CFE_Status_t)0xce000004)

/**
* @brief Bad Argument
*
* A parameter given by a caller to a TIME Services API did not pass
* validation checks.
*
*/
#define CFE_TIME_BAD_ARGUMENT ((CFE_Status_t)0xce000005)
/**@}*/

/* Compatibility for error names which have been updated */
Expand Down
6 changes: 6 additions & 0 deletions fsw/cfe-core/src/sb/cfe_sb_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -2128,9 +2128,15 @@ CFE_SB_Buffer_t *CFE_SB_ZeroCopyGetPtr(size_t MsgSize,
AppId = CFE_ES_APPID_UNDEFINED;
BufDscPtr = NULL;
BufPtr = NULL;
if(MsgSize > CFE_MISSION_SB_MAX_SB_MSG_SIZE)
{
CFE_ES_WriteToSysLog(" CFE_SB:ZeroCopyGetPtr-Failed, MsgSize is too large\n");
return NULL;
}

if (BufferHandle == NULL)
{
CFE_ES_WriteToSysLog(" CFE_SB:ZeroCopyGetPtr-BufferHandle is NULL\n");
return NULL;
}

Expand Down
82 changes: 58 additions & 24 deletions fsw/cfe-core/src/sb/cfe_sb_util.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
#include "osapi.h"
#include "cfe_error.h"
#include "cfe_msg_api.h"
#include "cfe_es.h"

#include <string.h>

Expand Down Expand Up @@ -75,6 +76,11 @@ size_t CFE_SB_MsgHdrSize(const CFE_MSG_Message_t *MsgPtr)
bool hassechdr = false;
CFE_MSG_Type_t type = CFE_MSG_Type_Invalid;

if (MsgPtr == NULL)
{
return CFE_SB_BAD_ARGUMENT;
}

CFE_MSG_GetHasSecondaryHeader(MsgPtr, &hassechdr);
CFE_MSG_GetType(MsgPtr, &type);

Expand Down Expand Up @@ -106,6 +112,11 @@ void *CFE_SB_GetUserData(CFE_MSG_Message_t *MsgPtr)
uint8 *BytePtr;
size_t HdrSize;

if(MsgPtr == NULL){
CFE_ES_WriteToSysLog("CFE_SB:GetUserData-Failed invalid arguments\n");
return 0;
}

BytePtr = (uint8 *)MsgPtr;
HdrSize = CFE_SB_MsgHdrSize(MsgPtr);

Expand All @@ -121,6 +132,11 @@ size_t CFE_SB_GetUserDataLength(const CFE_MSG_Message_t *MsgPtr)
CFE_MSG_Size_t TotalMsgSize;
size_t HdrSize;

if (MsgPtr == NULL)
{
return CFE_SB_BAD_ARGUMENT;
}

CFE_MSG_GetSize(MsgPtr, &TotalMsgSize);
HdrSize = CFE_SB_MsgHdrSize(MsgPtr);

Expand All @@ -136,11 +152,22 @@ void CFE_SB_SetUserDataLength(CFE_MSG_Message_t *MsgPtr, size_t DataLength)
CFE_MSG_Size_t TotalMsgSize;
size_t HdrSize;

HdrSize = CFE_SB_MsgHdrSize(MsgPtr);
TotalMsgSize = HdrSize + DataLength;
if(MsgPtr == NULL){
CFE_ES_WriteToSysLog("CFE_SB:SetUserDataLength-Failed invalid arguments\n");
}
else
{
HdrSize = CFE_SB_MsgHdrSize(MsgPtr);
TotalMsgSize = HdrSize + DataLength;

CFE_MSG_SetSize(MsgPtr, TotalMsgSize);

if(TotalMsgSize <= CFE_MISSION_SB_MAX_SB_MSG_SIZE){
CFE_MSG_SetSize(MsgPtr, TotalMsgSize);
}
else
{
CFE_ES_WriteToSysLog("CFE_SB:SetUserDataLength-Failed TotalMsgSize too large\n");
}
}
}/* end CFE_SB_SetUserDataLength */

#ifndef CFE_OMIT_DEPRECATED_6_8
Expand Down Expand Up @@ -288,7 +315,7 @@ int32 CFE_SB_MessageStringGet(char *DestStringPtr, const char *SourceStringPtr,
* Cannot terminate the string, since there is no place for the NUL
* In this case, do nothing
*/
if (DestMaxSize == 0)
if (DestMaxSize == 0 || DestStringPtr == NULL )
{
Result = CFE_SB_BAD_ARGUMENT;
}
Expand Down Expand Up @@ -335,28 +362,35 @@ int32 CFE_SB_MessageStringSet(char *DestStringPtr, const char *SourceStringPtr,
{
int32 Result;

Result = 0;

while (SourceMaxSize > 0 && *SourceStringPtr != 0 && DestMaxSize > 0)
if (SourceStringPtr == NULL || DestStringPtr == NULL )
{
*DestStringPtr = *SourceStringPtr;
++DestStringPtr;
++SourceStringPtr;
++Result;
--DestMaxSize;
--SourceMaxSize;
Result = CFE_SB_BAD_ARGUMENT;
}

/*
* Pad the remaining space with NUL chars,
* but this should NOT be included in the final size
*/
while (DestMaxSize > 0)
else
{
/* Put the NUL in the last character */
*DestStringPtr = 0;
++DestStringPtr;
--DestMaxSize;
Result = 0;

while (SourceMaxSize > 0 && *SourceStringPtr != 0 && DestMaxSize > 0)
{
*DestStringPtr = *SourceStringPtr;
++DestStringPtr;
++SourceStringPtr;
++Result;
--DestMaxSize;
--SourceMaxSize;
}

/*
* Pad the remaining space with NUL chars,
* but this should NOT be included in the final size
*/
while (DestMaxSize > 0)
{
/* Put the NUL in the last character */
*DestStringPtr = 0;
++DestStringPtr;
--DestMaxSize;
}
}

return Result;
Expand Down
Loading

0 comments on commit b50ae63

Please sign in to comment.