TakeV/dxx-rebirth
1
0
Fork 0
Code Issues Pull requests Packages Projects Releases Wiki Activity

Fix off-by-strlen(sep) in buffer capacity check

Browse source 
JoeNotCharles reports that gcc-12 (architecture unspecified) warns about
a possible truncation when constructing a path.  This would only occur
if the user had a path that was almost PATH_MAX deep (4096 bytes on most
platforms).  Add safety checks around this, and around an underflow if
this path were somehow reached while `newpath` was shorter than `sep`.
This is loosely based on a proposed commit by JoeNotCharles, but
rewritten to update comments and adjust the code flow.

Reported-by: JoeNotCharles <edfb3c4b77>
This commit is contained in:
Kp 2023-01-14 19:05:37 +00:00
parent 9839ca185b
commit b8f25c9d1e
1 changed files with 12 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
similar/main/menu.cpp
View file

@ -2334,10 +2334,19 @@ window_event_result browser::callback_handler(const d_event &event, window_event
{
const size_t len_newpath = strlen(newpath.data());
const size_t len_item = strlen(list[citem]);
if (len_newpath + len_item < newpath.size())
/* The separator may or may not be included, depending on
* whether one is already present. Regardless of whether a new
* separator is printed by the block, require sufficient space
* for it to fit. This is pessimistic, but should discourage
* gcc's -Wformat-truncation from warning about insufficient
* buffer capacity. In practice, users should never have a
* path that approaches this limit, so the `if` test should
* succeed in all reasonable cases.
*/
const size_t len_sep = strlen(sep);
if (len_newpath + len_item + len_sep < newpath.size())
{
const size_t len_sep = strlen(sep);
snprintf(&newpath[len_newpath], newpath.size() - len_newpath, "%s%s", strncmp(&newpath[len_newpath - len_sep], sep, len_sep) ? sep : "", list[citem]);
snprintf(&newpath[len_newpath], newpath.size() - len_newpath, "%s%s", (len_newpath >= len_sep && strncmp(&newpath[len_newpath - len_sep], sep, len_sep)) ? sep : "", list[citem]);
}
}
if ((citem == 0) || PHYSFS_isDirectory(list[citem]))