From cf25a50281e40e90d05b195cba3ae349fe7df700 Mon Sep 17 00:00:00 2001 From: Alex Campbell Date: Thu, 28 Jan 2021 11:16:49 -0500 Subject: [PATCH] Fix #547, add null pointer checks --- fsw/cfe-core/src/es/cfe_es_api.c | 92 +++++++++++++++++++--------- fsw/cfe-core/src/es/cfe_es_mempool.c | 20 ++++++ fsw/cfe-core/src/evs/cfe_evs.c | 12 ++++ fsw/cfe-core/src/fs/cfe_fs_api.c | 23 ++++++- fsw/cfe-core/src/inc/cfe_error.h | 17 +++++ fsw/cfe-core/src/sb/cfe_sb_api.c | 8 ++- fsw/cfe-core/src/sb/cfe_sb_util.c | 76 +++++++++++++++-------- fsw/cfe-core/src/tbl/cfe_tbl_api.c | 29 +++++++++ fsw/cfe-core/src/time/cfe_time_api.c | 16 +++++ 9 files changed, 236 insertions(+), 57 deletions(-) diff --git a/fsw/cfe-core/src/es/cfe_es_api.c b/fsw/cfe-core/src/es/cfe_es_api.c index 916264a1d..f22a9e366 100644 --- a/fsw/cfe-core/src/es/cfe_es_api.c +++ b/fsw/cfe-core/src/es/cfe_es_api.c @@ -1602,6 +1602,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); @@ -1633,6 +1638,11 @@ uint32 CFE_ES_CalculateCRC(const void *DataPtr, size_t DataLength, uint32 InputC const uint8 *BufPtr; uint8 ByteValue; + if (DataPtr == NULL || DataLength == 0) + { + return InputCRC; + } + static const uint16 CrcTable[256]= { @@ -1727,46 +1737,60 @@ int32 CFE_ES_RegisterCDS(CFE_ES_CDSHandle_t *CDSHandlePtr, size_t BlockSize, con /* Check to make sure calling application is legit */ Status = CFE_ES_GetAppID(&ThisAppId); - if ( Status != CFE_SUCCESS ) /* Application ID was invalid */ - { - CFE_ES_WriteToSysLog("CFE_CDS:Register-Bad AppId context\n"); - } - else if (!CFE_ES_Global.CDSIsAvailable) + if ( Status != CFE_SUCCESS || CDSHandlePtr == NULL || Name == NULL) /* Application ID was invalid */ { - CFE_ES_WriteToSysLog("CFE_CDS:Register-CDS not available\n"); - Status = CFE_ES_NOT_IMPLEMENTED; + CFE_ES_WriteToSysLog("CFE_ES_RegisterCDS:-Failed invalid arguments\n"); + Status = CFE_ES_BAD_ARGUMENT; } 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)) - { - Status = CFE_ES_CDS_INVALID_NAME; + /* Initialize output to safe value, in case this fails */ + *CDSHandlePtr = CFE_ES_RESOURCEID_UNDEFINED; - /* Perform a buffer overrun safe copy of name for debug log message */ + /* Check to make sure calling application is legit */ + Status = CFE_ES_GetAppID(&ThisAppId); - strncpy(CDSName, Name, sizeof(CDSName) - 1); - CDSName[sizeof(CDSName) - 1] = '\0'; - CFE_ES_WriteToSysLog("CFE_CDS:Register-CDS Name (%s) is too long\n", CDSName); + if ( Status != CFE_SUCCESS ) /* Application ID was invalid */ + { + CFE_ES_WriteToSysLog("CFE_CDS:Register-Bad AppId context\n"); + } + else if (!CFE_ES_Global.CDSIsAvailable) + { + CFE_ES_WriteToSysLog("CFE_CDS:Register-CDS not available\n"); + Status = CFE_ES_NOT_IMPLEMENTED; } else { - /* Modify specified name to be processor specific name */ - /* of the form "AppName.Name" */ - CFE_ES_FormCDSName(CDSName, Name, ThisAppId); + /* Assume we can't make a CDS and return a bad handle for now */ + *CDSHandlePtr = CFE_ES_CDS_BAD_HANDLE; - /* Create CDS and designate it as NOT being a Critical Table */ - Status = CFE_ES_RegisterCDSEx(CDSHandlePtr, BlockSize, CDSName, false); + /* 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)) + { + Status = CFE_ES_CDS_INVALID_NAME; - /* If size is unacceptable, log it */ - if (Status == CFE_ES_CDS_INVALID_SIZE) - { - CFE_ES_WriteToSysLog("CFE_CDS:Register-CDS %s has invalid size (%lu)\n", Name, (unsigned long)BlockSize); - } + /* Perform a buffer overrun safe copy of name for debug log message */ + + strncpy(CDSName, Name, sizeof(CDSName) - 1); + CDSName[sizeof(CDSName) - 1] = '\0'; + CFE_ES_WriteToSysLog("CFE_CDS:Register-CDS Name (%s) is too long\n", CDSName); + } + else + { + /* Modify specified name to be processor specific name */ + /* of the form "AppName.Name" */ + CFE_ES_FormCDSName(CDSName, Name, ThisAppId); + + /* Create CDS and designate it as NOT being a Critical Table */ + Status = CFE_ES_RegisterCDSEx(CDSHandlePtr, BlockSize, CDSName, false); + + /* If size is unacceptable, log it */ + if (Status == CFE_ES_CDS_INVALID_SIZE) + { + CFE_ES_WriteToSysLog("CFE_CDS:Register-CDS %s has invalid size (%lu)\n", Name, (unsigned long)BlockSize); + } + } } } @@ -1875,6 +1899,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() */ @@ -1886,6 +1915,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() */ diff --git a/fsw/cfe-core/src/es/cfe_es_mempool.c b/fsw/cfe-core/src/es/cfe_es_mempool.c index 5832b128d..66a833a42 100644 --- a/fsw/cfe-core/src/es/cfe_es_mempool.c +++ b/fsw/cfe-core/src/es/cfe_es_mempool.c @@ -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 */ @@ -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 */ @@ -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 */ @@ -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 */ diff --git a/fsw/cfe-core/src/evs/cfe_evs.c b/fsw/cfe-core/src/evs/cfe_evs.c index f3fb70b34..ac6c32318 100644 --- a/fsw/cfe-core/src/evs/cfe_evs.c +++ b/fsw/cfe-core/src/evs/cfe_evs.c @@ -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) @@ -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) { @@ -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) diff --git a/fsw/cfe-core/src/fs/cfe_fs_api.c b/fsw/cfe-core/src/fs/cfe_fs_api.c index 4f83e5f13..5f00d21e0 100644 --- a/fsw/cfe-core/src/fs/cfe_fs_api.c +++ b/fsw/cfe-core/src/fs/cfe_fs_api.c @@ -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... @@ -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; + } } /* @@ -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... */ diff --git a/fsw/cfe-core/src/inc/cfe_error.h b/fsw/cfe-core/src/inc/cfe_error.h index 872c89f03..4b1231b10 100644 --- a/fsw/cfe-core/src/inc/cfe_error.h +++ b/fsw/cfe-core/src/inc/cfe_error.h @@ -1325,6 +1325,14 @@ typedef int32 CFE_Status_t; */ #define CFE_TBL_ERR_ACCESS ((int32)0xcc00002c) +/** + * @brief Bad Argument + * + * A parameter given by a caller to a Table API did not pass + * validation checks. + * + */ +#define CFE_TBL_BAD_ARGUMENT ((int32)0xcc00002d) /** * @brief Not Implemented @@ -1400,6 +1408,15 @@ typedef int32 CFE_Status_t; * */ #define CFE_TIME_CALLBACK_NOT_REGISTERED ((int32)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 ((int32)0xce000005) /**@}*/ diff --git a/fsw/cfe-core/src/sb/cfe_sb_api.c b/fsw/cfe-core/src/sb/cfe_sb_api.c index 772a84e53..bd4908ef3 100644 --- a/fsw/cfe-core/src/sb/cfe_sb_api.c +++ b/fsw/cfe-core/src/sb/cfe_sb_api.c @@ -2147,7 +2147,13 @@ CFE_SB_Buffer_t *CFE_SB_ZeroCopyGetPtr(size_t MsgSize, CFE_ES_GetAppID(&AppId); zcd->AppID = AppId; - (*BufferHandle) = (CFE_SB_ZeroCopyHandle_t) zcd; + if(BufferHandle != NULL){ + (*BufferHandle) = (CFE_SB_ZeroCopyHandle_t) zcd; + } + else + { + CFE_ES_WriteToSysLog(" CFE_SB:ZeroCopyGetPtr-BufferHandle is NULL\n"); + } /* Initialize the buffer descriptor structure. */ bd->UseCount = 1; diff --git a/fsw/cfe-core/src/sb/cfe_sb_util.c b/fsw/cfe-core/src/sb/cfe_sb_util.c index 654f5f47b..119c067b7 100644 --- a/fsw/cfe-core/src/sb/cfe_sb_util.c +++ b/fsw/cfe-core/src/sb/cfe_sb_util.c @@ -39,6 +39,7 @@ #include "osapi.h" #include "cfe_error.h" #include "cfe_msg_api.h" +#include "cfe_es.h" #include @@ -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); @@ -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); @@ -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); @@ -136,11 +152,16 @@ 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); - + CFE_MSG_SetSize(MsgPtr, TotalMsgSize); + } }/* end CFE_SB_SetUserDataLength */ #ifndef CFE_OMIT_DEPRECATED_6_8 @@ -288,7 +309,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; } @@ -335,28 +356,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; diff --git a/fsw/cfe-core/src/tbl/cfe_tbl_api.c b/fsw/cfe-core/src/tbl/cfe_tbl_api.c index 6e70c020e..1b58a2e69 100644 --- a/fsw/cfe-core/src/tbl/cfe_tbl_api.c +++ b/fsw/cfe-core/src/tbl/cfe_tbl_api.c @@ -66,6 +66,11 @@ int32 CFE_TBL_Register( CFE_TBL_Handle_t *TblHandlePtr, char TblName[CFE_TBL_MAX_FULL_NAME_LEN] = {""}; CFE_TBL_Handle_t AccessIndex; + if (TblHandlePtr == NULL || Name == NULL) + { + return CFE_TBL_BAD_ARGUMENT; + } + /* Check to make sure calling application is legit */ Status = CFE_ES_GetAppID(&ThisAppId); @@ -522,6 +527,11 @@ int32 CFE_TBL_Share( CFE_TBL_Handle_t *TblHandlePtr, CFE_TBL_RegistryRec_t *RegRecPtr = NULL; char AppName[OS_MAX_API_NAME] = {"UNKNOWN"}; + if (TblHandlePtr == NULL || TblName == NULL) + { + return CFE_TBL_BAD_ARGUMENT; + } + /* Get a valid Application ID for calling App */ Status = CFE_ES_GetAppID(&ThisAppId); @@ -692,6 +702,11 @@ int32 CFE_TBL_Load( CFE_TBL_Handle_t TblHandle, char AppName[OS_MAX_API_NAME] = {"UNKNOWN"}; bool FirstTime = false; + if (SrcDataPtr == NULL) + { + return CFE_TBL_BAD_ARGUMENT; + } + /* Verify access rights and get a valid Application ID for calling App */ Status = CFE_TBL_ValidateAccess(TblHandle, &ThisAppId); @@ -1000,6 +1015,11 @@ int32 CFE_TBL_GetAddress( void **TblPtr, int32 Status; CFE_ES_AppId_t ThisAppId; + if (TblPtr == NULL) + { + return CFE_TBL_BAD_ARGUMENT; + } + /* Assume failure at returning the table address */ *TblPtr = NULL; @@ -1066,6 +1086,11 @@ int32 CFE_TBL_GetAddresses( void **TblPtrs[], int32 Status; CFE_ES_AppId_t ThisAppId; + if (TblPtrs == NULL) + { + return CFE_TBL_BAD_ARGUMENT; + } + /* Assume failure at returning the table addresses */ for (i=0; i