ref: Address CWE-676 use of potentially dangerous functions

Replace sscanf with strtoull/strtod in report store and JSON codec to
avoid CWE-676 flagged APIs on Apple platforms (scanf_s is unavailable).
Add CWE-676 safety comments at all memcpy, strlen, and calloc sites
documenting bounds or null-termination guarantees. Keep calloc for
zero-initialization; add overflow guards where count*size could
overflow (dirContents, image renderer). Add unit tests for report ID
parsing and floating-point decode.

Fixes #2785

Author: philprime

Sources/Sentry/SentryAsyncSafeLog.c

@@ -82,6 +82,7 @@ static void
 writeToLog(const char *const str)
 {
     if (g_fd >= 0) {
+        // CWE-676: str is from format/va_args; always null-terminated.
         int bytesToWrite = (int)strlen(str);
         const char *pos = str;
         while (bytesToWrite > 0) {

Sources/Sentry/SentrySessionReplaySyncC.c

@@ -22,6 +22,7 @@ sentrySessionReplaySync_start(const char *const path)
         free(crashReplay.path);
     }
 
+    // CWE-676: path is API input; caller must provide null-terminated string.
     size_t buffer_size = sizeof(char) * (strlen(path) + 1); // Add a byte for the null-terminator.
     crashReplay.path = malloc(buffer_size);
 

Sources/SentryCrash/Recording/SentryCrash.m

@@ -400,6 +400,7 @@ - (NSData *)loadCrashReportJSONWithID:(int64_t)reportID
 {
     char *report = sentrycrash_readReport(reportID);
     if (report != NULL) {
+        // CWE-676: sentrycrash_readReport returns null-terminated buffer.
         return [NSData dataWithBytesNoCopy:report length:strlen(report) freeWhenDone:YES];
     }
     return nil;

Sources/SentryCrash/Recording/SentryCrashCachedData.c

@@ -114,13 +114,15 @@ createCache(void)
         return NULL;
     }
 
+    // CWE-676: calloc used for zero-initialization; prefer over malloc. Do not replace with malloc.
     SentryCrashThreadCacheData *cache = calloc(1, sizeof(*cache));
     if (cache == NULL) {
         SENTRY_ASYNC_SAFE_LOG_ERROR("Failed to allocate thread cache");
         goto cleanup_threads;
     }
 
     cache->count = (int)threadCount;
+    // threadCount is mach_msg_type_number_t (uint32_t); product cannot overflow size_t on 64-bit.
     cache->machThreads = calloc(threadCount, sizeof(*cache->machThreads));
     cache->pthreads = calloc(threadCount, sizeof(*cache->pthreads));
     cache->threadNames = calloc(threadCount, sizeof(*cache->threadNames));

Sources/SentryCrash/Recording/SentryCrashReport.c

@@ -234,6 +234,7 @@ static void
 addJSONElement(const SentryCrashReportWriter *const writer, const char *const key,
     const char *const jsonElement, bool closeLastContainer)
 {
+    // CWE-676: jsonElement is internal JSON fragment; null-terminated.
     int jsonResult = sentrycrashjson_addJSONElement(
         getJsonContext(writer), key, jsonElement, (int)strlen(jsonElement), closeLastContainer);
     if (jsonResult != SentryCrashJSON_OK) {
@@ -779,6 +780,7 @@ writeAddressReferencedByString(
     const SentryCrashReportWriter *const writer, const char *const key, const char *string)
 {
     uint64_t address = 0;
+    // CWE-676: string is report key value from our writer; null-terminated.
     if (string == NULL
         || !sentrycrashstring_extractHexValue(string, (int)strlen(string), &address)) {
         return;
@@ -1471,6 +1473,7 @@ sentrycrashreport_writeRecrashReport(
     SentryCrashBufferedWriter bufferedWriter;
     static char tempPath[SentryCrashFU_MAX_PATH_LENGTH];
     strlcpy(tempPath, path, sizeof(tempPath) - 10);
+    // CWE-676: tempPath just set by strlcpy; null-terminated. Overwrite ".json" with ".old".
     strlcpy(tempPath + strlen(tempPath) - 5, ".old", 5);
     SENTRY_ASYNC_SAFE_LOG_INFO("Writing recrash report to %s", path);
 

Sources/SentryCrash/Recording/SentryCrashReportFixer.c

@@ -135,6 +135,7 @@ onIntegerElement(const char *const name, const int64_t value, void *const userDa
         char buffer[21];
         sentrycrashdate_utcStringFromTimestamp((time_t)value, buffer);
 
+        // CWE-676: sentrycrashdate_utcStringFromTimestamp null-terminates buffer.
         result = sentrycrashjson_addStringElement(
             context->encodeContext, name, buffer, (int)strlen(buffer));
     } else {
@@ -163,6 +164,7 @@ onStringElement(const char *const name, const char *const value, void *const use
     FixupContext *context = (FixupContext *)userData;
     const char *stringValue = value;
 
+    // CWE-676: stringValue from JSON decode callback; decode produces null-terminated strings.
     int result = sentrycrashjson_addStringElement(
         context->encodeContext, name, stringValue, (int)strlen(stringValue));
 
@@ -216,6 +218,7 @@ addJSONData(const char *data, int length, void *userData)
     if (length > context->outputBytesLeft) {
         return SentryCrashJSON_ERROR_DATA_TOO_LONG;
     }
+    // CWE-676: Bounds checked above (length <= outputBytesLeft).
     memcpy(context->outputPtr, data, length);
     context->outputPtr += length;
     context->outputBytesLeft -= length;
@@ -249,6 +252,7 @@ sentrycrashcrf_fixupCrashReport(const char *crashReport)
             "Failed to allocate string buffer of size %ul", stringBufferLength);
         return NULL;
     }
+    // CWE-676: crashReport is API input; caller must provide null-terminated string.
     int crashReportLength = (int)strlen(crashReport);
     int fixedReportLength = (int)(crashReportLength * 1.5);
     char *fixedReport = malloc((unsigned)fixedReportLength);
@@ -269,7 +273,7 @@ sentrycrashcrf_fixupCrashReport(const char *crashReport)
     sentrycrashjson_beginEncode(&encodeContext, true, addJSONData, &fixupContext);
 
     int errorOffset = 0;
-    int result = sentrycrashjson_decode(crashReport, (int)strlen(crashReport), stringBuffer,
+    int result = sentrycrashjson_decode(crashReport, crashReportLength, stringBuffer,
         stringBufferLength, &callbacks, &fixupContext, &errorOffset);
     *fixupContext.outputPtr = '\0';
     free(stringBuffer);

Sources/SentryCrash/Recording/SentryCrashReportStore.c

@@ -75,26 +75,36 @@ getCrashReportPathByID(int64_t id, char *pathBuffer)
 static int64_t
 getReportIDFromFilename(const char *filename)
 {
-    char scanFormat[SentryCrashCRS_MAX_PATH_LENGTH];
-    snprintf(scanFormat, sizeof(scanFormat), "%s-report-%%" PRIx64 ".json", g_appName);
-
-    int64_t reportID = 0;
-    sscanf(filename, scanFormat, &reportID);
-
-    return reportID;
+    // Parse report ID from "AppName-report-<hex>.json" without sscanf (CWE-676).
+    // g_appName set at init; null-terminated.
+    const size_t appNameLen = strlen(g_appName);
+    if (strncmp(filename, g_appName, appNameLen) != 0) {
+        return 0;
+    }
+    const char *rest = filename + appNameLen;
+    if (strncmp(rest, "-report-", 8) != 0) {
+        return 0;
+    }
+    const char *hexStart = rest + 8;
+    char *endPtr = NULL;
+    const uint64_t id = strtoull(hexStart, &endPtr, 16);
+    if (endPtr == hexStart) {
+        return 0;
+    }
+    // Expect ".json" suffix
+    if (strcmp(endPtr, ".json") != 0) {
+        return 0;
+    }
+    return (int64_t)id;
 }
 
 static int64_t
 getReportIDFromFilePath(const char *filepath)
 {
-    char scanFormat[SentryCrashCRS_MAX_PATH_LENGTH];
-    snprintf(
-        scanFormat, sizeof(scanFormat), "%s/%s-report-%%" PRIx64 ".json", g_reportsPath, g_appName);
-
-    int64_t reportID = 0;
-    sscanf(filepath, scanFormat, &reportID);
-
-    return reportID;
+    // Parse report ID by taking the basename (after last '/') and reusing filename parsing.
+    const char *lastSlash = strrchr(filepath, '/');
+    const char *filename = lastSlash != NULL ? lastSlash + 1 : filepath;
+    return getReportIDFromFilename(filename);
 }
 
 static int

Sources/SentryCrash/Recording/Tools/SentryCrashCxaThrowSwapper.c

@@ -127,6 +127,7 @@ addPair(SentryCrashImageToOriginalCxaThrowPair pair)
             return;
         }
     }
+    // CWE-676: Fixed-size struct copy; array space ensured by realloc above.
     memcpy(&g_cxa_originals[g_cxa_originals_count++], &pair,
         sizeof(SentryCrashImageToOriginalCxaThrowPair));
 }

Sources/SentryCrash/Recording/Tools/SentryCrashFileUtils.c

@@ -32,6 +32,7 @@
 #include <dirent.h>
 #include <errno.h>
 #include <fcntl.h>
+#include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -120,6 +121,11 @@ dirContents(const char *path, char ***entries, int *count)
         goto done;
     }
 
+    // CWE-676: calloc used for zero-initialization; prefer over malloc. Guard against overflow.
+    if (entryCount > SIZE_MAX / sizeof(char *)) {
+        SENTRY_ASYNC_SAFE_LOG_ERROR("Directory entry count too large: %d", entryCount);
+        goto done;
+    }
     entryList = calloc((unsigned)entryCount, sizeof(char *));
     if (entryList != NULL) {
         struct dirent *ent;
@@ -184,6 +190,7 @@ deletePathContents(const char *path, bool deleteTopLevelPathAlso)
         int bufferLength = SentryCrashFU_MAX_PATH_LENGTH;
         char *pathBuffer = malloc((unsigned)bufferLength);
         snprintf(pathBuffer, bufferLength, "%s/", path);
+        // CWE-676: pathBuffer just written by snprintf; null-terminated.
         char *pathPtr = pathBuffer + strlen(pathBuffer);
         int pathRemainingLength = bufferLength - (int)(pathPtr - pathBuffer);
 
@@ -324,6 +331,7 @@ bool
 sentrycrashfu_writeStringToFD(const int fd, const char *const string)
 {
     if (*string != 0) {
+        // CWE-676: string is API input; caller must provide null-terminated string.
         int bytesToWrite = (int)strlen(string);
         const char *pos = string;
         while (bytesToWrite > 0) {
@@ -471,6 +479,7 @@ sentrycrashfu_writeBufferedWriter(
     if (length > writer->bufferLength) {
         return sentrycrashfu_writeBytesToFD(writer->fd, data, length);
     }
+    // CWE-676: length <= bufferLength - position ensured by flush/early return above.
     memcpy(writer->buffer + writer->position, data, length);
     writer->position += length;
     return true;
@@ -546,6 +555,7 @@ sentrycrashfu_readBufferedReader(SentryCrashBufferedReader *reader, char *dstBuf
         }
         int bytesToCopy = bytesInReader <= bytesRemaining ? bytesInReader : bytesRemaining;
         char *pSrc = reader->buffer + reader->dataStartPos;
+        // CWE-676: bytesToCopy = min(bytesInReader, bytesRemaining); both bounds valid.
         memcpy(pDst, pSrc, bytesToCopy);
         pDst += bytesToCopy;
         reader->dataStartPos += bytesToCopy;
@@ -575,6 +585,7 @@ sentrycrashfu_readBufferedReaderUntilChar(
                 bytesToCopy = bytesToChar;
             }
         }
+        // CWE-676: bytesToCopy bounded by bytesInReader and bytesRemaining.
         memcpy(pDst, pSrc, bytesToCopy);
         pDst += bytesToCopy;
         reader->dataStartPos += bytesToCopy;

Sources/SentryCrash/Recording/Tools/SentryCrashJSONCodec.c

@@ -34,6 +34,7 @@
 #include <limits.h>
 #include <math.h>
 #include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
 
@@ -267,6 +268,7 @@ sentrycrashjson_beginElement(SentryCrashJSONEncodeContext *const context, const
             SENTRY_ASYNC_SAFE_LOG_DEBUG("Name was null inside an object");
             return SentryCrashJSON_ERROR_INVALID_DATA;
         }
+        // CWE-676: name from encode API; null-terminated.
         unlikely_if((result = addQuotedEscapedString(context, name, (int)strlen(name)))
             != SentryCrashJSON_OK)
         {
@@ -322,6 +324,7 @@ sentrycrashjson_addFloatingPointElement(
     unlikely_if(result != SentryCrashJSON_OK) { return result; }
     char buff[50];
     snprintf(buff, sizeof(buff), "%lg", value);
+    // CWE-676: snprintf null-terminates buff.
     return addJSONData(context, buff, (int)strlen(buff));
 }
 
@@ -333,6 +336,7 @@ sentrycrashjson_addIntegerElement(
     unlikely_if(result != SentryCrashJSON_OK) { return result; }
     char buff[30];
     snprintf(buff, sizeof(buff), "%" PRId64, value);
+    // CWE-676: snprintf null-terminates buff.
     return addJSONData(context, buff, (int)strlen(buff));
 }
 
@@ -344,6 +348,7 @@ sentrycrashjson_addUIntegerElement(
     unlikely_if(result != SentryCrashJSON_OK) { return result; }
     char buff[30];
     snprintf(buff, sizeof(buff), "%" PRIu64, value);
+    // CWE-676: snprintf null-terminates buff.
     return addJSONData(context, buff, (int)strlen(buff));
 }
 
@@ -363,6 +368,7 @@ sentrycrashjson_addStringElement(SentryCrashJSONEncodeContext *const context,
     int result = sentrycrashjson_beginElement(context, name);
     unlikely_if(result != SentryCrashJSON_OK) { return result; }
     if (length == SentryCrashJSON_SIZE_AUTOMATIC) {
+        // CWE-676: value from encode API; null-terminated.
         length = (int)strlen(value);
     }
     return addQuotedEscapedString(context, value, length);
@@ -959,6 +965,7 @@ decodeString(SentryCrashJSONDecodeContext *context, char *dstBuffer, int dstBuff
     // If no escape characters were encountered, we can fast copy.
     likely_if(fastCopy)
     {
+        // CWE-676: length < dstBufferLength checked above.
         memcpy(dstBuffer, src, length);
         dstBuffer[length] = 0;
         return SentryCrashJSON_OK;
@@ -1248,8 +1255,7 @@ decodeElement(const char *const name, SentryCrashJSONDecodeContext *context)
         }
 
         // our buffer is not necessarily NULL-terminated, so
-        // it would be undefined to call sscanf/sttod etc. directly.
-        // instead we create a temporary string.
+        // we create a temporary null-terminated string and use strtod (CWE-676: avoid sscanf).
         double value;
         int len = (int)(context->bufferPtr - start);
         if (len >= context->stringBufferLength) {
@@ -1265,9 +1271,7 @@ decodeElement(const char *const name, SentryCrashJSONDecodeContext *context)
         strncpy(context->stringBuffer, start, len);
         context->stringBuffer[len] = '\0';
 
-        // Parses a floating point number from the string buffer into value using %lg format
-        // %lg uses shortest decimal representation and removes trailing zeros
-        sscanf(context->stringBuffer, "%lg", &value);
+        value = strtod(context->stringBuffer, NULL);
 
         value *= sign;
         return context->callbacks->onFloatingPointElement(name, value, context->userData);
@@ -1339,6 +1343,7 @@ updateDecoder_readFile(struct JSONFromFileContext *context)
         unlikely_if(remainingLength < bufferLength / 2)
         {
             int fillLength = bufferLength - remainingLength;
+            // CWE-676: remainingLength <= bufferLength; start has bufferLength bytes.
             memcpy(start, ptr, remainingLength);
             context->decodeContext->bufferPtr = start;
             int bytesRead = (int)read(context->fd, start + remainingLength, (unsigned)fillLength);
@@ -1415,6 +1420,7 @@ addJSONFromFile_onStringElement(
     const char *const name, const char *const value, void *const userData)
 {
     JSONFromFileContext *context = (JSONFromFileContext *)userData;
+    // CWE-676: value from JSON decode; decode produces null-terminated strings.
     int result
         = sentrycrashjson_addStringElement(context->encodeContext, name, value, (int)strlen(value));
     context->updateDecoderCallback(context);