Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Sources/Sentry/SentryAsyncSafeLog.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions Sources/Sentry/SentrySessionReplaySyncC.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
1 change: 1 addition & 0 deletions Sources/SentryCrash/Recording/SentryCrash.m
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions Sources/SentryCrash/Recording/SentryCrashCachedData.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
3 changes: 3 additions & 0 deletions Sources/SentryCrash/Recording/SentryCrashReport.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down
6 changes: 5 additions & 1 deletion Sources/SentryCrash/Recording/SentryCrashReportFixer.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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));

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down
40 changes: 25 additions & 15 deletions Sources/SentryCrash/Recording/SentryCrashReportStore.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
11 changes: 11 additions & 0 deletions Sources/SentryCrash/Recording/Tools/SentryCrashFileUtils.c
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
16 changes: 11 additions & 5 deletions Sources/SentryCrash/Recording/Tools/SentryCrashJSONCodec.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include <limits.h>
#include <math.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>

Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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));
}

Expand All @@ -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));
}

Expand All @@ -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));
}

Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to double-check if this is truly the same


value *= sign;
return context->callbacks->onFloatingPointElement(name, value, context->userData);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ sentrycrashmc_getContextForSignal(
SENTRY_ASYNC_SAFE_LOG_DEBUG(
"Get context from signal user context and put into %p.", destinationContext);
_STRUCT_MCONTEXT *sourceContext = ((SignalUserContext *)signalUserContext)->UC_MCONTEXT;
// CWE-676: Fixed-size copy; source and destination are same struct type.
memcpy(&destinationContext->machineContext, sourceContext,
sizeof(destinationContext->machineContext));
destinationContext->thisThread = (thread_t)sentrycrashthread_self();
Expand Down
3 changes: 3 additions & 0 deletions Sources/SentryCrash/Recording/Tools/SentryCrashObjC.c
Original file line number Diff line number Diff line change
Expand Up @@ -1104,13 +1104,15 @@ sentrycrashobjc_ivarValue(const void *const objectPtr, int ivarIndex, void *dst)
// Naively assume they want "value".
if (isTaggedPointerNSDate(objectPtr)) {
CFTimeInterval value = extractTaggedNSDate(objectPtr);
// CWE-676: Fixed-size copy; caller provides at least sizeof(value) bytes.
memcpy(dst, &value, sizeof(value));
return true;
}
if (isTaggedPointerNSNumber(objectPtr)) {
// TODO: Correct to assume 64-bit signed int? What does the actual
// ivar say?
int64_t value = extractTaggedNSNumber(objectPtr);
// CWE-676: Fixed-size copy; caller provides at least sizeof(value) bytes.
memcpy(dst, &value, sizeof(value));
return true;
}
Expand Down Expand Up @@ -1550,6 +1552,7 @@ taggedDateDescription(const void *object, char *buffer, int bufferLength)
#define NSNUMBER_CASE(CFTYPE, RETURN_TYPE, CAST_TYPE, DATA) \
case CFTYPE: { \
RETURN_TYPE result; \
/* CWE-676: DATA is __CFNumber _pad; fixed-size copy matching RETURN_TYPE. */ \
memcpy(&result, DATA, sizeof(result)); \
return (CAST_TYPE)result; \
}
Expand Down
1 change: 1 addition & 0 deletions Sources/SentryCrash/Recording/Tools/SentryCrashSysCtl.c
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ sentrycrashsysctl_getMacAddress(const char *const name, char *const macAddressBu

struct if_msghdr *msgHdr = (struct if_msghdr *)ifBuffer;
struct sockaddr_dl *sockaddr = (struct sockaddr_dl *)&msgHdr[1];
// CWE-676: MAC address is always 6 bytes; caller provides 6-byte buffer (see header).
memcpy(macAddressBuffer, LLADDR(sockaddr), 6);

free(ifBuffer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,15 @@ final class SentryGraphicsImageRenderer {
let bytesPerRow = bytesPerPixel * pixelsPerRow
let bitsPerComponent = 8 // 8 bits for each of RGB component

// Allocate memory for raw image data and initializes every byte in the allocated memory to 0.
guard let rawData = calloc(pixelsPerColumn * bytesPerRow, MemoryLayout<UInt8>.size) else {
SentrySDKLog.error("Unable to allocate memory for image data")
// CWE-676: calloc used for zero-initialization; prefer over malloc. Guard against overflow.
let byteCount = pixelsPerColumn.multipliedReportingOverflow(by: bytesPerRow)
guard !byteCount.overflow, byteCount.partialValue > 0,
let rawData = calloc(byteCount.partialValue, MemoryLayout<UInt8>.size) else {
if byteCount.overflow {
SentrySDKLog.error("Image dimensions cause allocation overflow")
} else {
SentrySDKLog.error("Unable to allocate memory for image data")
}
return UIImage()
}
defer {
Expand Down
Loading
Loading