I ran the Fortify Static Code Analyzer on the ossec-hids repo and it reported the following "Buffer Overflow: Format String" finding for src/analysisd/stats.c:415:
The format string argument to fscanf() at stats.c line 415 does not properly limit the amount of data the function can write, which allows the program to write outside the bounds of allocated memory. This behavior could corrupt data, crash the program, or lead to the execution of malicious code
The line of code in question is like this:
if (fscanf(fp, "%d", &_RWHour[i][j]) <= 0) {
_RWHour is declared as
static int _RWHour[7][25];
on line 33 of the same file. I believe there is no shadowing of _RWHour between this declaration on line 33 and its use on line 415, as when I select the declaration on line 33, my IDE (Visual Studio 2019) highlights _RWHour in 415.
When I look at the cppreference documentation for fscanf, it says the following:
d matches a decimal integer. The format of the number is the same as expected by
strtolwith the value10for the base argument
The table I took the quote above from also shows that when no length modifier is used for %d (as is the case for the fscanf call in question), the argument type should be signed int* or unsigned int*.
My question is this:
Is it possible for the Fortify finding to be a false positive? Or is it possible to write to memory outside an int when you pass the address of an int to fscanf?
If it is possible to write outside the memory of an int when using %d with fscanf, how can this be safely avoided?
2
Answers
fscanf(fp, "%d", &_RWHour[i][j])is undefined behavior (UB)*1 if the numeric text attempts to convert to a value outside theintrange.A quick fix to prevent UB is to limit the number of characters read with a width:
A more robust solution reads in the text and uses
strtol()to convert.I recommend creating a helper function to handle reading in the
int.This level of checking kinda stinks, doesn’t it?
*1
With UB, it is "possible to write to memory outside an int".
Title: Can fscanf buffer overflow when using %d?
Question:
I am using
fscanfin C to read integers from a file using the%dformat specifier. I have heard about buffer overflows withscanf, but canfscanfalso be vulnerable to buffer overflow when reading integers with%d? Should I be concerned about the safety of my code?Answer:
The
%dformat specifier infscanfis generally safe from buffer overflows when used correctly. It reads an integer value from the input and stores it into the provided variable.Unlike
scanf, which reads from standard input (stdin),fscanfreads from a file stream, which can help mitigate some potential risks. When usingfscanfwith a file stream, the risk of buffer overflow is reduced because the data is read from a predefined file, rather than from user input.However, it’s important to ensure that your code handles potential errors or unexpected input appropriately. Here are a few points to consider:
Buffer Size: Make sure the variable you are reading the integer into has sufficient space to accommodate the largest possible value. If the buffer size is not large enough, it could lead to undefined behavior, including buffer overflow.
Input Validation: Validate the input to ensure it matches the expected format. If the input does not conform to the
%dformat specifier, the corresponding variable may not be updated correctly, potentially leading to unexpected behavior or uninitialized values.Return Value: Check the return value of
fscanfto verify if the expected number of items were successfully read. If the return value differs from what you expect, handle the situation accordingly.By following these best practices and properly handling input and output, you can minimize the risk of buffer overflows or other unexpected behavior when using
fscanfwith the%dformat specifier.Remember that while
%ditself does not pose a significant buffer overflow risk, other format specifiers, such as%sfor strings, can be vulnerable if not used carefully. Always be mindful of the data you are reading, the buffer sizes, and the overall security of your code.