Can `snprintf()` Read Out Of Bounds When A String Argument Is Not Null-terminated?
I have the following piece of code, which a colleague claims may contain an out-of-bounds read, which I do not agree with. Could you help settle this argument and explain why?
char *test_filename = malloc(Size + 1); sprintf(test_filename, ""); if (Size > 0 && Data) snprintf(test_filename, Size + 1, "%s", Data);
Data is a non-null-terminated string of type
const uint8_t *Data and
Size is the size of
Data, i.e., number of bytes in
Data, of type
It may read out-of-bounds because the format string is
Your colleague is correct. Perhaps unintuitively,
snprintf(test_filename, Size + 1, "%s", Data) is guaranteed to read bytes starting at
Data until a 0 byte is encountered, in your case typically resulting in an out-of-bounds read.
It will only write
Size of these bytes to
test_filename and null terminate them, respecting the size limit of the destination; but it will continue to read on. The reason for that is a design choice which enables the caller to determine the needed destination size for dynamic allocation before anything is actually written:
snprintf() returns the number of bytes which would be written if the destination had infinite space. This feature is supposed to be used with a destination size of 0 (and potentially a null pointer as the destination). This functionality is useful for arguments which are not strings: With numbers etc. the size of the output is difficult to predict (e.g. locale dependent) and best left to the function at run time.
At the same time the return value indicates whether the output was truncated: If it is greater or equal to the size parameter, not all of the input was used in the output. In your case, what was left out were the bytes starting a
Data[Size] and ending with the first 0 byte, or a segmentation fault ;-).
Suggestion for a fix: First of all it is unclear why you would use the
printf family to print a string; simply copy it. And then Andrew has a point in his comments that since
Datais not null terminated it is not really a string (even if all bytes are printable); so don't start fiddling with
strcpy and friends but simply
memcpy() the bytes, and null terminate manually.
Oh, and the preceding
sprintf(test_filename, ""); does not serve any discernible purpose. If you want to write a null byte to
*Data, simply do so; but since you are not using
strcat, which would rely on a terminated destination string to extend, it is quite unnecessary.
- → OctoberCMS Backend Loging Hash Error
- → "failed to open stream" error when executing "migrate:make"
- → OctoberCMS - How to make collapsible list default to active only on non-mobile
- → Create plugin that makes objects from model in back-end
- → October CMS Plugin Routes.php not registering
- → OctoberCMS Migrate Table
- → How to install console for plugin development in October CMS
- → OctoberCMS Rain User plugin not working or redirecting
- → October CMS Custom Mail Layout
- → October CMS - How to correctly route
- → October CMS create a multi select Form field
- → How to update data attribute on Ajax complete
- → October CMS - Conditionally Load a Different Page