Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
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