[hardening] TBL load integer overflow + memory-safety cleanups

modules/es/fsw/src/cfe_es_cds.c

@@ -672,8 +672,11 @@ void CFE_ES_FormCDSName(char *FullCDSName, const char *CDSName, CFE_ES_AppId_t T
     /* Ensure that AppName is null terminated */
     AppName[OS_MAX_API_NAME - 1] = '\0';
 
-    /* Complete formation of processor specific table name */
-    sprintf(FullCDSName, "%s.%s", AppName, CDSName);
+    /* Complete formation of processor specific table name.
+     * Use snprintf with the documented full-name length so that any future
+     * widening of OS_MAX_API_NAME or CFE_MISSION_ES_CDS_MAX_NAME_LENGTH
+     * cannot silently overflow the caller-supplied buffer. */
+    snprintf(FullCDSName, CFE_MISSION_ES_CDS_MAX_FULL_NAME_LEN, "%s.%s", AppName, CDSName);
 }
 
 /*----------------------------------------------------------------

modules/es/fsw/src/cfe_es_mempool.c

@@ -59,6 +59,49 @@
 ** Functions
 */
 
+/*----------------------------------------------------------------
+ *
+ * Internal helper — take a pool mutex if defined, log and proceed on failure.
+ * Matches the "log and continue" convention used by CFE_SB_LockSharedData /
+ * CFE_FS_LockSharedData / CFE_TBL_LockRegistry so a transient OSAL error does
+ * not silently mask an unprotected critical section.
+ *
+ *-----------------------------------------------------------------*/
+static inline void CFE_ES_PoolLock(const CFE_ES_MemPoolRecord_t *PoolRecPtr, const char *FuncName)
+{
+    int32 OsStatus;
+
+    if (OS_ObjectIdDefined(PoolRecPtr->MutexId))
+    {
+        OsStatus = OS_MutSemTake(PoolRecPtr->MutexId);
+        if (OsStatus != OS_SUCCESS)
+        {
+            CFE_ES_WriteToSysLog("%s: PoolMutex Take Err Stat=%ld,Pool=%lu\n", FuncName, (long)OsStatus,
+                                 OS_ObjectIdToInteger(PoolRecPtr->MutexId));
+        }
+    }
+}
+
+/*----------------------------------------------------------------
+ *
+ * Internal helper — release a pool mutex if defined, log on failure.
+ *
+ *-----------------------------------------------------------------*/
+static inline void CFE_ES_PoolUnlock(const CFE_ES_MemPoolRecord_t *PoolRecPtr, const char *FuncName)
+{
+    int32 OsStatus;
+
+    if (OS_ObjectIdDefined(PoolRecPtr->MutexId))
+    {
+        OsStatus = OS_MutSemGive(PoolRecPtr->MutexId);
+        if (OsStatus != OS_SUCCESS)
+        {
+            CFE_ES_WriteToSysLog("%s: PoolMutex Give Err Stat=%ld,Pool=%lu\n", FuncName, (long)OsStatus,
+                                 OS_ObjectIdToInteger(PoolRecPtr->MutexId));
+        }
+    }
+}
+
 /*----------------------------------------------------------------
  *
  * Internal helper routine only, not part of API.
@@ -447,10 +490,7 @@ int32 CFE_ES_GetPoolBuf(CFE_ES_MemPoolBuf_t *BufPtr, CFE_ES_MemHandle_t Handle,
      * Real work begins here.
      * If pool is mutex-protected, take the mutex now.
      */
-    if (OS_ObjectIdDefined(PoolRecPtr->MutexId))
-    {
-        OS_MutSemTake(PoolRecPtr->MutexId);
-    }
+    CFE_ES_PoolLock(PoolRecPtr, __func__);
 
     /*
      * Fundamental work is done as a generic routine.
@@ -464,10 +504,7 @@ int32 CFE_ES_GetPoolBuf(CFE_ES_MemPoolBuf_t *BufPtr, CFE_ES_MemHandle_t Handle,
      * Real work ends here.
      * If pool is mutex-protected, release the mutex now.
      */
-    if (OS_ObjectIdDefined(PoolRecPtr->MutexId))
-    {
-        OS_MutSemGive(PoolRecPtr->MutexId);
-    }
+    CFE_ES_PoolUnlock(PoolRecPtr, __func__);
 
     /* If not successful, return error now */
     if (Status != CFE_SUCCESS)
@@ -511,10 +548,7 @@ CFE_Status_t CFE_ES_GetPoolBufInfo(CFE_ES_MemHandle_t Handle, CFE_ES_MemPoolBuf_
      * Real work begins here.
      * If pool is mutex-protected, take the mutex now.
      */
-    if (OS_ObjectIdDefined(PoolRecPtr->MutexId))
-    {
-        OS_MutSemTake(PoolRecPtr->MutexId);
-    }
+    CFE_ES_PoolLock(PoolRecPtr, __func__);
 
     DataOffset = (cpuaddr)BufPtr - PoolRecPtr->BaseAddr;
 
@@ -524,10 +558,7 @@ CFE_Status_t CFE_ES_GetPoolBufInfo(CFE_ES_MemHandle_t Handle, CFE_ES_MemPoolBuf_
      * Real work ends here.
      * If pool is mutex-protected, release the mutex now.
      */
-    if (OS_ObjectIdDefined(PoolRecPtr->MutexId))
-    {
-        OS_MutSemGive(PoolRecPtr->MutexId);
-    }
+    CFE_ES_PoolUnlock(PoolRecPtr, __func__);
 
     if (Status == CFE_SUCCESS)
     {
@@ -573,10 +604,7 @@ int32 CFE_ES_PutPoolBuf(CFE_ES_MemHandle_t Handle, CFE_ES_MemPoolBuf_t BufPtr)
      * Real work begins here.
      * If pool is mutex-protected, take the mutex now.
      */
-    if (OS_ObjectIdDefined(PoolRecPtr->MutexId))
-    {
-        OS_MutSemTake(PoolRecPtr->MutexId);
-    }
+    CFE_ES_PoolLock(PoolRecPtr, __func__);
 
     DataOffset = (cpuaddr)BufPtr - PoolRecPtr->BaseAddr;
 
@@ -592,10 +620,7 @@ int32 CFE_ES_PutPoolBuf(CFE_ES_MemHandle_t Handle, CFE_ES_MemPoolBuf_t BufPtr)
      * Real work ends here.
      * If pool is mutex-protected, release the mutex now.
      */
-    if (OS_ObjectIdDefined(PoolRecPtr->MutexId))
-    {
-        OS_MutSemGive(PoolRecPtr->MutexId);
-    }
+    CFE_ES_PoolUnlock(PoolRecPtr, __func__);
 
     /*
      * If successful then modify return code to be
@@ -653,10 +678,7 @@ CFE_Status_t CFE_ES_GetMemPoolStats(CFE_ES_MemPoolStats_t *BufPtr, CFE_ES_MemHan
      * Real work begins here.
      * If pool is mutex-protected, take the mutex now.
      */
-    if (OS_ObjectIdDefined(PoolRecPtr->MutexId))
-    {
-        OS_MutSemTake(PoolRecPtr->MutexId);
-    }
+    CFE_ES_PoolLock(PoolRecPtr, __func__);
 
     /*
      * Obtain the free and total byte count
@@ -682,10 +704,7 @@ CFE_Status_t CFE_ES_GetMemPoolStats(CFE_ES_MemPoolStats_t *BufPtr, CFE_ES_MemHan
      * Real work ends here.
      * If pool is mutex-protected, release the mutex now.
      */
-    if (OS_ObjectIdDefined(PoolRecPtr->MutexId))
-    {
-        OS_MutSemGive(PoolRecPtr->MutexId);
-    }
+    CFE_ES_PoolUnlock(PoolRecPtr, __func__);
 
     return CFE_SUCCESS;
 }

modules/sb/fsw/src/cfe_sb_priv.c

@@ -279,7 +279,11 @@ char *CFE_SB_GetAppTskName(CFE_ES_TaskId_t TaskId, char *FullName)
         strncpy(TskName, (char *)ptr->TaskName, sizeof(TskName) - 1);
         TskName[sizeof(TskName) - 1] = '\0';
 
-        sprintf(FullName, "%s.%s", AppName, TskName);
+        /* Use snprintf with the caller-documented buffer width.  All callers
+         * declare FullName as char[OS_MAX_API_NAME * 2], so the cap below
+         * matches that contract and protects against any future widening of
+         * OS_MAX_API_NAME or change in the AppName/TskName field sizes. */
+        snprintf(FullName, OS_MAX_API_NAME * 2, "%s.%s", AppName, TskName);
     }
 
     return FullName;

modules/tbl/fsw/src/cfe_tbl_eds_codec.c

@@ -411,7 +411,18 @@ CFE_Status_t CFE_TBL_ValidateCodecLoadSize(CFE_TBL_TxnState_t *Txn, const CFE_TB
 
     if (Status == CFE_SUCCESS)
     {
-        ProjectedSize = HeaderPtr->Offset + HeaderPtr->NumBytes;
+        /* Check Offset+NumBytes for unsigned overflow before performing the addition.
+         * Both fields originate from a (potentially attacker-controlled) file header,
+         * so a wrapped sum could otherwise bypass the size check below. */
+        if (HeaderPtr->NumBytes > (UINT32_MAX - HeaderPtr->Offset))
+        {
+            ProjectedSize = UINT32_MAX;
+        }
+        else
+        {
+            ProjectedSize = HeaderPtr->Offset + HeaderPtr->NumBytes;
+        }
+
         if (ProjectedSize > ActualSize)
         {
             Status = CFE_TBL_ERR_FILE_TOO_LARGE;

modules/tbl/fsw/src/cfe_tbl_internal.c

@@ -867,15 +867,39 @@ CFE_Status_t CFE_TBL_AllocateTableLoadBuffer(CFE_TBL_LoadBuff_t *LoadBuffPtr, si
  *-----------------------------------------------------------------*/
 void CFE_TBL_MarkNameAsModified(char *NameBufPtr, size_t NameBufSize)
 {
-    char *EndPtr;
+    static const char Suffix[]  = "(*)";
+    const size_t      SuffixLen = sizeof(Suffix) - 1; /* 3, excludes NUL */
+    char *            EndPtr;
+    size_t            EndOffset;
+
+    /* Buffer must hold suffix + NUL.  Anything smaller is a programming error;
+     * silently doing nothing is safer than truncation that would not actually
+     * mark the name. */
+    if (NameBufPtr == NULL || NameBufSize <= SuffixLen)
+    {
+        return;
+    }
 
-    EndPtr = memchr(NameBufPtr, 0, NameBufSize);
-    if ((EndPtr - NameBufPtr) > (NameBufSize - 4))
+    /* Find the existing NUL terminator.  If none is present within NameBufSize
+     * (caller error / corrupted name), append the suffix at a position that
+     * leaves room for it plus a final NUL. */
+    EndPtr = memchr(NameBufPtr, '\0', NameBufSize);
+    if (EndPtr == NULL)
     {
-        EndPtr = &NameBufPtr[NameBufSize - 4];
+        EndOffset = NameBufSize - SuffixLen - 1;
+    }
+    else
+    {
+        EndOffset = (size_t)(EndPtr - NameBufPtr);
+        if (EndOffset > NameBufSize - SuffixLen - 1)
+        {
+            EndOffset = NameBufSize - SuffixLen - 1;
+        }
     }
 
-    strcpy(EndPtr, "(*)");
+    /* memcpy suffix + explicit NUL — avoids strcpy and any string.h length scan. */
+    memcpy(&NameBufPtr[EndOffset], Suffix, SuffixLen);
+    NameBufPtr[EndOffset + SuffixLen] = '\0';
 }
 
 /*----------------------------------------------------------------

modules/tbl/fsw/src/cfe_tbl_load.c

@@ -248,8 +248,22 @@ CFE_Status_t CFE_TBL_LoadContentFromFile(CFE_TBL_TxnState_t *Txn, osal_id_t File
         return Status;
     }
 
+    /* Compute the tail of the load region. Both Offset and NumBytes originate from
+     * a (potentially attacker-controlled) file header, so guard against unsigned
+     * overflow before the addition.  On 32-bit targets size_t is 32 bits and the
+     * sum can wrap; on wider hosts this guard is still cheap and preserves identical
+     * behavior for valid input. If overflow would occur, force LoadTailSize to
+     * SIZE_MAX so the existing oversize check below rejects the load. */
+    if (NumBytes > (SIZE_MAX - Offset))
+    {
+        LoadTailSize = SIZE_MAX;
+    }
+    else
+    {
+        LoadTailSize = Offset + NumBytes;
+    }
+
     /* Confirm that the data about to be loaded will fit */
-    LoadTailSize = Offset + NumBytes;
     if (LoadTailSize > CFE_TBL_LoadBuffGetAllocSize(WorkingBufferPtr))
     {
         Status = CFE_TBL_ERR_FILE_TOO_LARGE;

modules/tbl/fsw/src/cfe_tbl_passthru_codec.c

@@ -159,8 +159,20 @@ CFE_Status_t CFE_TBL_ValidateCodecLoadSize(CFE_TBL_TxnState_t *Txn, const CFE_TB
     uint32       ProjectedSize;
 
     /* Because the file sizes and in-memory size is the same, this check is simple */
-    ActualSize    = CFE_TBL_RegRecGetSize(CFE_TBL_TxnRegRec(Txn));
-    ProjectedSize = HeaderPtr->Offset + HeaderPtr->NumBytes;
+    ActualSize = CFE_TBL_RegRecGetSize(CFE_TBL_TxnRegRec(Txn));
+
+    /* Check Offset+NumBytes for unsigned overflow before performing the addition.
+     * Both fields originate from a (potentially attacker-controlled) file header,
+     * so a wrapped sum could otherwise bypass the size check below. */
+    if (HeaderPtr->NumBytes > (UINT32_MAX - HeaderPtr->Offset))
+    {
+        ProjectedSize = UINT32_MAX;
+    }
+    else
+    {
+        ProjectedSize = HeaderPtr->Offset + HeaderPtr->NumBytes;
+    }
+
     if (ProjectedSize > ActualSize)
     {
         Status = CFE_TBL_ERR_FILE_TOO_LARGE;