Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't crash when using readdir_r() if name_max is determined to be 0 #1903

Merged
merged 2 commits into from Jul 13, 2023
Merged

Don't crash when using readdir_r() if name_max is determined to be 0 #1903

merged 2 commits into from Jul 13, 2023

Conversation

lukem
Copy link
Contributor

@lukem lukem commented Jun 10, 2023

Avoid a crash when the memory allocated for the struct dirent written to by readdir_r() isn't large enough because filesystem.name_max==0 (as copied from struct statfs or struct statvfs), so there's a memory overwrite.

This should fix issue #1149 and NetBSD PR https://gnats.netbsd.org/56080.

If using HAVE_STATVFS, set name_max from svfs.f_namelen
instead of sfs.f_namelen, to be consistent with the
rest of the function.

Noticed by code inspection.
Add error handling to the USE_READDIR_R code paths that set name_max
from struct statfs or statvfs; if the determined name_max == 0
then return an error.

Avoids a crash in tree_dir_next_posix() when the calculation of
dirent_size from name_max is too small for the memory allocated
for struct dirent.

This may fix Github issue #1149
This may fix NetBSD PR https://gnats.netbsd.org/56080
@lukem lukem mentioned this pull request Jun 10, 2023
@lukem
Copy link
Contributor Author

lukem commented Jun 10, 2023

Arguably the error paths I added could instead attempt to fall back to NAME_MAX (if defined) or PATH_MAX, similar to the "generic" setup_current_filesystem() variant which has the code:

#  if defined(NAME_MAX) && NAME_MAX >= 255
                t->current_filesystem->name_max = NAME_MAX;
#  else
                /* No way to get a trusted value of maximum filename
                 * length. */
                t->current_filesystem->name_max = PATH_MAX;
#  endif /* NAME_MAX */

I wasn't sure what the preferred technique was from the libarchive maintainers, so I chose to just fail hard instead.
If the above workaround is preferred I can revise the PR.

netbsd-srcmastr pushed a commit to NetBSD/src that referenced this pull request Jun 10, 2023
Add error handling to the USE_READDIR_R code paths that set name_max
from struct statfs or statvfs; if the determined name_max == 0
then return an error.

Avoids a crash in tree_dir_next_posix() when the calculation of
dirent_size from name_max is too small for the memory allocated
for struct dirent.

Submitted to upstream in pull request
	libarchive/libarchive#1903

Should fix PR bin/56080
@mmatuska mmatuska merged commit dc56a48 into libarchive:master Jul 13, 2023
4 of 6 checks passed
@lukem lukem deleted the lukem-name_max-checks branch July 15, 2023 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants