We've identified an issue in versions 1.x.x where crashes occasionally occur during the JSON serialization of crash reports. The problem appears to be a potential buffer overflow vulnerability in our floating point value handling.
This function uses sprintf, which lacks bounds checking and could lead to buffer overflows. To address this, we propose replacing sprintf with snprintf. This change would provide proper bounds checking and prevent potential buffer overflows.
Here's the proposed modification:
int ksjson_addFloatingPointElement(KSJSONEncodeContext* const context, const char* const name, double value)
{
int result = ksjson_beginElement(context, name);
unlikely_if(result != KSJSON_OK) { return result; }
char buff[30];
int written = snprintf(buff, sizeof(buff), "%lg", value);
if (written < 0 || written >= sizeof(buff)) {
// Handle error or truncation
return KSJSON_ERROR_INVALID_DATA;
}
return addJSONData(context, buff, written);
}
It's important to note that this is only one of several places where sprintf is used. We should conduct a thorough review of the codebase and replace all instances of sprintf with safer alternatives like snprintf to prevent similar vulnerabilities elsewhere.
For context, here's a stack trace from one of the crashes:
We've identified an issue in versions 1.x.x where crashes occasionally occur during the JSON serialization of crash reports. The problem appears to be a potential buffer overflow vulnerability in our floating point value handling.
One of the functions affected is: https://github.com/kstenerud/KSCrash/blob/8cc2170a9d3bc7775b71311bff231e600ca1349e/Sources/KSCrashRecordingCore/KSJSONCodec.c#L275-L282
This function uses
sprintf, which lacks bounds checking and could lead to buffer overflows. To address this, we propose replacingsprintfwithsnprintf. This change would provide proper bounds checking and prevent potential buffer overflows.Here's the proposed modification:
It's important to note that this is only one of several places where
sprintfis used. We should conduct a thorough review of the codebase and replace all instances ofsprintfwith safer alternatives likesnprintfto prevent similar vulnerabilities elsewhere.For context, here's a stack trace from one of the crashes: