mirror of
https://github.com/zlib-ng/minizip-ng
synced 2026-08-25 14:26:08 -04:00
fix: reject symlink targets that escape extraction dir
Symlink archive entries had their stored target passed directly to mz_os_make_symlink without checking containment, so a malicious archive could create a link pointing outside the extraction root. Add mz_path_is_symlink_target_safe to verify a link target resolves within the destination directory, and apply it to both the UNIX1 extrafield and entry-content link targets. Reject symlink entries outright when no destination base is set, since the target cannot be validated without one. Assisted-By: Claude Opus 4.8
This commit is contained in:
parent
181642d2f8
commit
131d59ba39
5 changed files with 158 additions and 49 deletions
28
doc/mz_os.md
28
doc/mz_os.md
|
|
@ -13,6 +13,7 @@ These functions provide support for handling common file system operations.
|
|||
- [mz\_path\_remove\_filename](#mz_path_remove_filename)
|
||||
- [mz\_path\_remove\_extension](#mz_path_remove_extension)
|
||||
- [mz\_path\_get\_filename](#mz_path_get_filename)
|
||||
- [mz\_path\_is\_symlink\_target\_safe](#mz_path_is_symlink_target_safe)
|
||||
- [Directory](#directory)
|
||||
- [mz\_dir\_has\_unsafe\_symlink](#mz_dir_has_unsafe_symlink)
|
||||
- [mz\_dir\_make](#mz_dir_make)
|
||||
|
|
@ -285,6 +286,33 @@ else
|
|||
printf("Path has no filename\n");
|
||||
```
|
||||
|
||||
### mz_path_is_symlink_target_safe
|
||||
|
||||
Checks if a symbolic link target resolves within a base path. Used to prevent a malicious archive from creating a symbolic link that points outside the extraction directory.
|
||||
|
||||
**Arguments**
|
||||
|Type|Name|Description|
|
||||
|-|-|-|
|
||||
|const char *|link_path|Path of the symbolic link to be created|
|
||||
|const char *|target|Symbolic link target|
|
||||
|const char *|base_path|Base path that the target must not escape|
|
||||
|
||||
**Return**
|
||||
|Type|Description|
|
||||
|-|-|
|
||||
|int32_t|[MZ_ERROR](mz_error.md) code, MZ_OK if the target is safe, MZ_EXIST_ERROR if it escapes the base path.|
|
||||
|
||||
**Example**
|
||||
```
|
||||
const char *base_path = "/tmp/extract/";
|
||||
const char *link_path = "/tmp/extract/link";
|
||||
const char *target = "../outside.txt";
|
||||
if (mz_path_is_symlink_target_safe(link_path, target, base_path) == MZ_OK)
|
||||
printf("Symlink target is safe to create\n");
|
||||
else
|
||||
printf("Symlink target escapes base path\n");
|
||||
```
|
||||
|
||||
## Directory
|
||||
|
||||
### mz_dir_has_unsafe_symlink
|
||||
|
|
|
|||
112
mz_os.c
112
mz_os.c
|
|
@ -277,15 +277,75 @@ int32_t mz_path_get_filename(const char *path, const char **filename) {
|
|||
return MZ_OK;
|
||||
}
|
||||
|
||||
int32_t mz_path_is_symlink_target_safe(const char *link_path, const char *target, const char *base_path) {
|
||||
char *combined = NULL;
|
||||
char *resolved = NULL;
|
||||
size_t max_path = 1024;
|
||||
size_t base_len = 0;
|
||||
size_t parent_len = 0;
|
||||
int32_t err = MZ_OK;
|
||||
|
||||
if (!link_path || !target || !base_path)
|
||||
return MZ_PARAM_ERROR;
|
||||
|
||||
/* Absolute symlink targets are not allowed */
|
||||
if (mz_os_is_dir_separator(target[0]))
|
||||
return MZ_EXIST_ERROR;
|
||||
|
||||
base_len = strlen(base_path);
|
||||
|
||||
/* Remove trailing slash from base_path for comparison */
|
||||
while (base_len > 0 && mz_os_is_dir_separator(base_path[base_len - 1]))
|
||||
base_len--;
|
||||
|
||||
combined = (char *)calloc(1, max_path);
|
||||
resolved = (char *)calloc(1, max_path);
|
||||
|
||||
if (!combined || !resolved) {
|
||||
err = MZ_MEM_ERROR;
|
||||
goto target_cleanup;
|
||||
}
|
||||
|
||||
/* Find parent directory length by scanning backwards past filename and trailing slashes */
|
||||
parent_len = strlen(link_path);
|
||||
while (parent_len > 0 && !mz_os_is_dir_separator(link_path[parent_len - 1]))
|
||||
parent_len--;
|
||||
while (parent_len > 0 && mz_os_is_dir_separator(link_path[parent_len - 1]))
|
||||
parent_len--;
|
||||
|
||||
/* Combine parent + target */
|
||||
combined[0] = 0;
|
||||
if (parent_len > 0) {
|
||||
strncpy(combined, link_path, parent_len);
|
||||
combined[parent_len] = 0;
|
||||
mz_path_append_slash(combined, (int32_t)max_path, MZ_PATH_SLASH_PLATFORM);
|
||||
}
|
||||
strncat(combined, target, max_path - strlen(combined) - 1);
|
||||
|
||||
/* Resolve the combined path to eliminate .. */
|
||||
if (mz_path_resolve(combined, resolved, (int32_t)max_path) != MZ_OK) {
|
||||
err = MZ_EXIST_ERROR;
|
||||
goto target_cleanup;
|
||||
}
|
||||
|
||||
/* Check that resolved path stays within base_path */
|
||||
if (strlen(resolved) < base_len || strncmp(resolved, base_path, base_len) != 0 ||
|
||||
(resolved[base_len] != 0 && !mz_os_is_dir_separator(resolved[base_len])))
|
||||
err = MZ_EXIST_ERROR;
|
||||
|
||||
target_cleanup:
|
||||
free(combined);
|
||||
free(resolved);
|
||||
|
||||
return err;
|
||||
}
|
||||
|
||||
int32_t mz_dir_has_unsafe_symlink(const char *path, const char *base_path) {
|
||||
char *check_path = NULL;
|
||||
char *symlink_target = NULL;
|
||||
char *combined = NULL;
|
||||
char *resolved = NULL;
|
||||
size_t path_len = 0;
|
||||
size_t base_len = 0;
|
||||
size_t max_path = 1024;
|
||||
size_t parent_len = 0;
|
||||
size_t pos = 0;
|
||||
size_t cmp_len = 0;
|
||||
int32_t err = MZ_OK;
|
||||
|
|
@ -333,13 +393,10 @@ int32_t mz_dir_has_unsafe_symlink(const char *path, const char *base_path) {
|
|||
continue;
|
||||
}
|
||||
|
||||
/* Allocate symlink buffers on first use */
|
||||
/* Allocate symlink target buffer on first use */
|
||||
if (!symlink_target) {
|
||||
symlink_target = (char *)calloc(1, max_path);
|
||||
combined = (char *)calloc(1, max_path);
|
||||
resolved = (char *)calloc(1, max_path);
|
||||
|
||||
if (!symlink_target || !combined || !resolved) {
|
||||
if (!symlink_target) {
|
||||
err = MZ_MEM_ERROR;
|
||||
break;
|
||||
}
|
||||
|
|
@ -350,47 +407,14 @@ int32_t mz_dir_has_unsafe_symlink(const char *path, const char *base_path) {
|
|||
break;
|
||||
}
|
||||
|
||||
/* Absolute symlink targets are not allowed */
|
||||
if (mz_os_is_dir_separator(symlink_target[0])) {
|
||||
err = MZ_EXIST_ERROR;
|
||||
/* Reject the component if its symlink target escapes the base path */
|
||||
err = mz_path_is_symlink_target_safe(check_path, symlink_target, base_path);
|
||||
if (err != MZ_OK)
|
||||
break;
|
||||
}
|
||||
|
||||
/* Find parent directory length by scanning backwards past filename and trailing slashes */
|
||||
parent_len = pos;
|
||||
while (parent_len > 0 && !mz_os_is_dir_separator(check_path[parent_len - 1]))
|
||||
parent_len--;
|
||||
while (parent_len > 0 && mz_os_is_dir_separator(check_path[parent_len - 1]))
|
||||
parent_len--;
|
||||
|
||||
/* Combine parent + symlink_target */
|
||||
combined[0] = 0;
|
||||
if (parent_len > 0) {
|
||||
strncpy(combined, check_path, parent_len);
|
||||
combined[parent_len] = 0;
|
||||
mz_path_append_slash(combined, (int32_t)max_path, MZ_PATH_SLASH_PLATFORM);
|
||||
}
|
||||
strncat(combined, symlink_target, max_path - strlen(combined) - 1);
|
||||
|
||||
/* Resolve the combined path to eliminate .. */
|
||||
if (mz_path_resolve(combined, resolved, (int32_t)max_path) != MZ_OK) {
|
||||
err = MZ_EXIST_ERROR;
|
||||
break;
|
||||
}
|
||||
|
||||
/* Check that resolved path starts with base_path */
|
||||
if (strlen(resolved) < base_len ||
|
||||
strncmp(resolved, base_path, base_len) != 0 ||
|
||||
(resolved[base_len] != 0 && !mz_os_is_dir_separator(resolved[base_len]))) {
|
||||
err = MZ_EXIST_ERROR;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
free(check_path);
|
||||
free(symlink_target);
|
||||
free(combined);
|
||||
free(resolved);
|
||||
|
||||
return err;
|
||||
}
|
||||
|
|
|
|||
3
mz_os.h
3
mz_os.h
|
|
@ -100,6 +100,9 @@ int32_t mz_path_remove_extension(char *path);
|
|||
int32_t mz_path_get_filename(const char *path, const char **filename);
|
||||
/* Get the filename from a path */
|
||||
|
||||
int32_t mz_path_is_symlink_target_safe(const char *link_path, const char *target, const char *base_path);
|
||||
/* Checks if a symlink target resolves within base path. */
|
||||
|
||||
int32_t mz_dir_has_unsafe_symlink(const char *path, const char *base_path);
|
||||
/* Checks if any existing component of path is a symlink that escapes base path. */
|
||||
|
||||
|
|
|
|||
16
mz_zip_rw.c
16
mz_zip_rw.c
|
|
@ -698,8 +698,7 @@ int32_t mz_zip_reader_entry_save_file(void *handle, const char *path) {
|
|||
}
|
||||
|
||||
/* Check if path traverses through an existing symlink that escapes destination */
|
||||
if (reader->destination_dir &&
|
||||
mz_dir_has_unsafe_symlink(directory, reader->destination_dir) != MZ_OK) {
|
||||
if (reader->destination_dir && mz_dir_has_unsafe_symlink(directory, reader->destination_dir) != MZ_OK) {
|
||||
err = MZ_EXIST_ERROR;
|
||||
goto save_cleanup;
|
||||
}
|
||||
|
|
@ -730,7 +729,10 @@ int32_t mz_zip_reader_entry_save_file(void *handle, const char *path) {
|
|||
if (mz_zip_entry_is_symlink(reader->zip_handle) == MZ_OK) {
|
||||
if (reader->file_info->linkname && *reader->file_info->linkname != 0) {
|
||||
/* Create symbolic link from UNIX1 extrafield */
|
||||
err = mz_os_make_symlink(pathwfs, reader->file_info->linkname);
|
||||
if (mz_path_is_symlink_target_safe(pathwfs, reader->file_info->linkname, reader->destination_dir) != MZ_OK)
|
||||
err = MZ_EXIST_ERROR;
|
||||
else
|
||||
err = mz_os_make_symlink(pathwfs, reader->file_info->linkname);
|
||||
} else if (reader->file_info->uncompressed_size < UINT16_MAX) {
|
||||
/* Create symbolic link from zip entry contents */
|
||||
stream = mz_stream_mem_create();
|
||||
|
|
@ -749,8 +751,12 @@ int32_t mz_zip_reader_entry_save_file(void *handle, const char *path) {
|
|||
|
||||
if (err == MZ_OK) {
|
||||
const char *linkname = NULL;
|
||||
if (mz_stream_mem_get_buffer(stream, (const void **)&linkname) == MZ_OK)
|
||||
err = mz_os_make_symlink(pathwfs, linkname);
|
||||
if (mz_stream_mem_get_buffer(stream, (const void **)&linkname) == MZ_OK) {
|
||||
if (mz_path_is_symlink_target_safe(pathwfs, linkname, reader->destination_dir) != MZ_OK)
|
||||
err = MZ_EXIST_ERROR;
|
||||
else
|
||||
err = mz_os_make_symlink(pathwfs, linkname);
|
||||
}
|
||||
}
|
||||
|
||||
mz_stream_mem_close(stream);
|
||||
|
|
|
|||
|
|
@ -65,3 +65,51 @@ TEST_P(path_resolve, os) {
|
|||
mz_path_resolve(path.c_str(), output, sizeof(output));
|
||||
EXPECT_STREQ(output, expected_path.c_str());
|
||||
}
|
||||
|
||||
struct symlink_base_param {
|
||||
const char *link_path;
|
||||
const char *target;
|
||||
const char *base_path;
|
||||
bool safe;
|
||||
|
||||
friend std::ostream &operator<<(std::ostream &os, const symlink_base_param ¶m) {
|
||||
return os << "link: " << param.link_path << " target: " << param.target;
|
||||
}
|
||||
};
|
||||
|
||||
constexpr symlink_base_param symlink_base_tests[] = {
|
||||
/* In-bounds targets are allowed */
|
||||
{ "base\\link", "inside.txt", "base", true},
|
||||
{ "base\\sub\\link", "..\\inside.txt", "base", true},
|
||||
{ "base\\link", "..\\base", "base", true},
|
||||
/* Targets that resolve outside the base are rejected */
|
||||
{ "base\\link", "..\\out.txt", "base", false},
|
||||
{"base\\a\\b\\link", "..\\..\\..\\out.txt", "base", false},
|
||||
/* Absolute targets are rejected */
|
||||
{ "base\\link", "\\etc\\passwd", "base", false},
|
||||
/* A sibling directory sharing the base name prefix is not in base */
|
||||
{ "base\\link", "..\\base_evil\\x", "base", false},
|
||||
};
|
||||
|
||||
class symlink_target_base : public ::testing::TestWithParam<symlink_base_param> {};
|
||||
|
||||
INSTANTIATE_TEST_SUITE_P(os, symlink_target_base, testing::ValuesIn(symlink_base_tests));
|
||||
|
||||
TEST_P(symlink_target_base, os) {
|
||||
const auto ¶m = GetParam();
|
||||
std::string link_path = param.link_path;
|
||||
std::string target = param.target;
|
||||
std::string base_path = param.base_path;
|
||||
|
||||
if (!mz_os_is_dir_separator('\\')) {
|
||||
std::replace(link_path.begin(), link_path.end(), '\\', '/');
|
||||
std::replace(target.begin(), target.end(), '\\', '/');
|
||||
std::replace(base_path.begin(), base_path.end(), '\\', '/');
|
||||
}
|
||||
|
||||
int32_t err = mz_path_is_symlink_target_safe(link_path.c_str(), target.c_str(), base_path.c_str());
|
||||
if (param.safe)
|
||||
EXPECT_EQ(err, MZ_OK);
|
||||
else
|
||||
EXPECT_NE(err, MZ_OK);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue