Skip to content

fix: add buffer-length check in unzip.cpp #429

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.

Sign up for GitHub

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

Open
anupamme wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: add buffer-length check in unzip.cpp #429

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a0ae972
fix: V-001 security vulnerability
anupamme Aug 4, 2026
da985c2
fix: eliminate fixed-size buffer in unzip.cpp to prevent path truncation
anupamme Aug 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions TKLiveSync/unzip.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#include "unzip.h"
#include "libzip/zip.h"
#include <assert.h>
#include <libgen.h>
#include <limits.h>
#include <string>
#include <sys/stat.h>
Expand Down Expand Up @@ -36,7 +35,6 @@ int64_t unzip(const char* syncZipPath, const char* destination)
struct zip_stat sb;
struct zip_file* zf;
char buf[65536];
auto pathcopy = new char[PATH_MAX];

for (zip_int64_t i = 0; i < num; i++) {
zip_stat_index(z, i, ZIP_STAT_MTIME, &sb);
Expand All @@ -46,12 +44,15 @@ int64_t unzip(const char* syncZipPath, const char* destination)
assetFullname.append("/");
assetFullname.append(name);

strcpy(pathcopy, name);
auto path = dirname(pathcopy);
std::string dirFullname(destination);
dirFullname.append("/");
dirFullname.append(path);
mkdir_rec(dirFullname.c_str());
std::string entryName{ name };
auto separator = entryName.find_last_of('/');

if (separator != std::string::npos) {
std::string dirFullname{ destination };
dirFullname.append("/");
dirFullname.append(entryName.substr(0, separator));
mkdir_rec(dirFullname.c_str());
}
Comment on lines +47 to +55

@coderabbitai coderabbitai Bot Aug 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Repository files relevant to unzip:"
git ls-files | rg '(^|/)unzip\.(cpp|h)$|TKLiveSync/TKLiveSync\.m|TKLiveSync/TKLiveSync\.mm$' || true

echo
echo "unzip.cpp outline:"
ast-grep outline TKLiveSync/unzip.cpp || true

echo
echo "unzip.cpp relevant lines:"
cat -n TKLiveSync/unzip.cpp | sed -n '1,120p'

echo
echo "unzip.h:"
cat -n TKLiveSync/unzip.h | sed -n '1,140p'

echo
echo "Path traversal behavior model for current construction:"
python3 - <<'PY'
from pathlib import PurePosixPath

def simulate(destination, name):
    separator = name.rfind("/")
    if separator != -1:
        dirFullname = destination + "/" + name[:separator]
    else:
        dirFullname = destination
    assetFullname = destination + "/" + name
    return dirFullname, assetFullname, PurePosixPath(assetFullname)

destination = "/Users/me/Application Support/LiveSync"
examples = ["../outside.txt", "../../outside.txt", "/tmp/outside.txt", "normal.txt", "subdir/../escape.txt"]
for name in examples:
    df, af, pp = simulate(destination, name)
    print(f"name={name!r}")
    print(f"  constructed_asset_fullname={af!r}')
    print(f"  posix_absolute={pp.is_absolute()}")
    print(f"  canonical_resolved_without_sysfs={pp.resolve() if str(pp).startswith('/') else 'depends on cwd'}")
    print(f"  startsWithDestinationCanonical=False")
PY

Repository: NativeScript/ios

Length of output: 3777

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import PurePosixPath

def simulate(destination, name):
    separator = name.rfind("/")
    if separator != -1:
        dirFullname = destination + "/" + name[:separator]
    else:
        dirFullname = destination
    assetFullname = destination + "/" + name
    return dirFullname, assetFullname, PurePosixPath(assetFullname)

destination = "/Users/me/Application Support/LiveSync"
examples = [
    "../outside.txt",
    "../../outside.txt",
    "/tmp/outside.txt",
    "normal.txt",
    "subdir/../escape.txt",
    "foo/../../escape.txt",
    "foo/bar/../../../escape.txt",
]
for name in examples:
    df, af, pp = simulate(destination, name)
    print(f"name={name!r}")
    print(f"  constructed_asset_fullname={af!r}")
    print(f"  posix_absolute={pp.is_absolute()}")
    print(f"  starts_with_traversal={name.startswith(('../', '/'))}")
PY

echo
echo "Search for unzip calls and LiveSync archive handling:"
rg -n "unzip\\(|sync\.zip|LiveSync" TKLiveSync

Repository: NativeScript/ios

Length of output: 2779

🌐 Web query:

libzip zip_get_name zip files ZIP filenames unsafe absolute paths parent directory traversal

💡 Result:

The use of filenames retrieved from ZIP archives—via functions like libzip's zip_get_name—poses a significant risk of directory traversal attacks [1][2][3]. This is not a vulnerability in libzip itself, but rather a standard security risk inherent in the ZIP file format, which allows entry names to contain absolute paths (e.g., /etc/passwd) or parent directory traversal sequences (e.g.,../../../) [1][4][3]. When an application uses these filenames to construct local file paths on the filesystem, it can lead to arbitrary file creation or overwriting outside the intended destination directory [1][2][5][3]. This class of vulnerability is commonly referred to as a ZipSlip attack [3]. To mitigate this risk, developers must treat any filename obtained from a ZIP archive as untrusted input [2][3]. You must validate and sanitize these paths before using them in filesystem operations [1][3]. Essential security practices include: 1. Canonicalization: Convert the target destination path and the extracted filename into their canonical forms to resolve any traversal sequences [3]. 2. Validation: After canonicalization, verify that the resulting path still resides within the intended destination directory (e.g., by checking if the canonical path starts with the prefix of the safe destination directory) [3]. 3. Disallow Absolute Paths: Explicitly check for and reject filenames that are absolute paths or attempt to escape the designated extraction root [1][3]. libzip provides the raw, unmodified names as stored in the archive [6][7]. It is the responsibility of the application developer to perform the necessary path normalization and security checks before opening or creating files based on these names [7][3].

Citations:

Path Traversal (CWE-22): Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')

Reachability: External

Treat archive entry names as untrusted before writing files.

sb.name is appended to both the directory path and assetFullname unchanged. Entries with .., /../, or absolute paths such as /tmp/outside.txt make macOS resolve paths back toward filesystem root for later segments, so unzip() can create files or overwrite contents outside the LiveSync destination. Reject absolute names and any .. path component before calling mkdir_rec() or fopen(), and fail the entry on invalid names.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@TKLiveSync/unzip.cpp` around lines 47 - 55, Validate each archive entry name
in unzip() before constructing directory paths or opening files: reject absolute
paths and any path component exactly equal to "..". Treat invalid names as
failed entries and skip further processing, ensuring mkdir_rec() and fopen() are
never called for them.


zf = zip_fopen_index(z, i, 0);
assert(zf != nullptr);
Expand All @@ -72,7 +73,6 @@ int64_t unzip(const char* syncZipPath, const char* destination)

zip_fclose(zf);
}
delete[] pathcopy;
zip_close(z);

return num;
Expand Down