Skip to content

Commit a05d868

Browse files
committed
fix: mcb_load_policy Windows absolute-path message mismatch (CI-red #3)
Root cause is std::filesystem::path::is_absolute() itself, not the test. On Windows it requires BOTH a root-name (drive letter/UNC) AND a root-directory; a POSIX-style rooted path like "/etc/passwd" has a root-directory but no root-name, so is_absolute() is false there even though it's exactly the filesystem-root-escape attempt the check exists to catch. The untrusted policy still rejected such a path on Windows (the includePathWithinRoot() fallback catches it as "escapes the document root" instead), just with a different message than mcb_load_policy's stricter substring assertion expected. mc3_load_policy/mc3_json_load_policy only assert that loading threw, not the message text, so they stayed green despite having the identical gap. Fixed all 3 mirrored is_absolute() checks (McbReader.cpp, Mc3XmlParser.cpp, Mc3JsonParser.cpp -- same names, same logic by design) to also treat "has a root-directory but no root-name" as absolute, so the rejection message is consistent on every platform. has_root_name() is always false on POSIX, so the added clause is already implied there by the existing is_absolute() check -- zero behavior change on Linux/macOS. Verified: full mc3_*/mcb_*/mc3togltf_* non-render suite (115/118) still passes unchanged under ASan+UBSan; the 3 failures are the already-known Blender/numpy environment gap, unrelated. Windows-side reasoning follows documented std::filesystem semantics, not executable in this sandbox (no Wine).
1 parent 1e7733d commit a05d868

4 files changed

Lines changed: 47 additions & 6 deletions

File tree

NEXT.md

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -176,9 +176,32 @@ place. Progress, each its own commit:
176176
non-render `mc3_*` suite (29 tests) pass clean under ASan+UBSan
177177
(`build-asan/`), no new leak/UB findings from the added `.close()` calls.
178178
Unverified on real Windows (no Wine in this sandbox).
179-
- Remaining: **#3** (`mcb_load_policy` POSIX- vs Windows-style absolute-path
180-
message text) and **#1** (`mc3togltf.exe` `STATUS_DLL_NOT_FOUND`, needs
181-
CMake DLL-staging/install-rule changes).
179+
- **#3** (`mcb_load_policy`'s POSIX- vs Windows-style absolute-path message
180+
text): root cause is `std::filesystem::path::is_absolute()` itself, not
181+
the test. On Windows it requires BOTH a root-name (drive letter/UNC) AND
182+
a root-directory; a POSIX-style rooted path like `/etc/passwd` has a
183+
root-directory but no root-name, so `is_absolute()` is false there even
184+
though it's exactly the filesystem-root-escape attempt the check exists
185+
to catch. The untrusted policy still rejected it on Windows (the
186+
`includePathWithinRoot()` fallback catches it as "escapes the document
187+
root" instead), just with a different message than the `mcb_load_policy`
188+
test's stricter substring assertion expected — `mc3_load_policy`/
189+
`mc3_json_load_policy`'s sibling tests only assert `threw`, not the
190+
message text, so they stayed green despite having the identical
191+
semantic gap. Fixed all 3 mirrored `is_absolute()` checks
192+
(`mcb/src/McbReader.cpp`, `mc3/src/Mc3XmlParser.cpp`,
193+
`mc3/src/Mc3JsonParser.cpp` — same names, same logic by design) to also
194+
treat "has a root-directory but no root-name" as absolute, so the
195+
rejection message is the same on every platform. `has_root_name()` is
196+
always false on POSIX, so `has_root_directory() && !has_root_name()` is
197+
already implied by the existing `is_absolute()` there — zero behavior
198+
change on Linux/macOS, confirmed by the full `mc3_*`/`mcb_*`/
199+
`mc3togltf_*` non-render suite (115/118 pass, the 3 failures are the
200+
already-known Blender/`numpy` gap, unrelated) still passing unchanged.
201+
Windows-side reasoning follows documented `std::filesystem` semantics
202+
(cppreference), not executable in this sandbox (no Wine).
203+
- Remaining: **#1** (`mc3togltf.exe` `STATUS_DLL_NOT_FOUND`, needs CMake
204+
DLL-staging/install-rule changes).
182205

183206
## Known release blockers and decisions
184207

mc3/src/Mc3JsonParser.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,13 @@ void validateResourcePathIfConfined(const std::string& rawPath, const char* kind
339339
if (rawPath.rfind("embed:", 0) == 0 || rawPath.rfind("data:", 0) == 0) return;
340340

341341
std::filesystem::path p(rawPath);
342-
if (p.is_absolute()) {
342+
// is_absolute() requires a root-name (e.g. a Windows drive letter) AND a
343+
// root-directory on Windows, so a POSIX-style rooted path like
344+
// "/etc/passwd" -- exactly the same filesystem-root escape attempt
345+
// is_absolute() exists to catch on POSIX -- is false there. Treat
346+
// root-directory-without-root-name as absolute too, so an untrusted
347+
// document is rejected with the same message on every platform.
348+
if (p.is_absolute() || (p.has_root_directory() && !p.has_root_name())) {
343349
std::string msg = std::string("MC3: ") + kind + " '" + rawPath +
344350
"' is an absolute path outside the document root; rejected "
345351
"under the untrusted-content load policy";

mc3/src/Mc3XmlParser.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1617,7 +1617,13 @@ static void validateResourcePathIfConfined(const XMLElement* el, const std::stri
16171617
if (rawPath.rfind("embed:", 0) == 0 || rawPath.rfind("data:", 0) == 0) return;
16181618

16191619
std::filesystem::path p(rawPath);
1620-
if (p.is_absolute()) {
1620+
// is_absolute() requires a root-name (e.g. a Windows drive letter) AND a
1621+
// root-directory on Windows, so a POSIX-style rooted path like
1622+
// "/etc/passwd" -- exactly the same filesystem-root escape attempt
1623+
// is_absolute() exists to catch on POSIX -- is false there. Treat
1624+
// root-directory-without-root-name as absolute too, so an untrusted
1625+
// document is rejected with the same message on every platform.
1626+
if (p.is_absolute() || (p.has_root_directory() && !p.has_root_name())) {
16211627
std::string msg = std::string("MC3: ") + kind + " '" + rawPath +
16221628
"' is an absolute path outside the document root; rejected under "
16231629
"the untrusted-content load policy";

mcb/src/McbReader.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,13 @@ static void validateResourcePathIfConfined(const std::string& rawPath, const cha
156156
if (rawPath.rfind("embed:", 0) == 0 || rawPath.rfind("data:", 0) == 0) return;
157157

158158
std::filesystem::path p(rawPath);
159-
if (p.is_absolute()) {
159+
// is_absolute() requires a root-name (e.g. a Windows drive letter) AND a
160+
// root-directory on Windows, so a POSIX-style rooted path like
161+
// "/etc/passwd" -- exactly the same filesystem-root escape attempt
162+
// is_absolute() exists to catch on POSIX -- is false there. Treat
163+
// root-directory-without-root-name as absolute too, so an untrusted MCB
164+
// document is rejected with the same message on every platform.
165+
if (p.is_absolute() || (p.has_root_directory() && !p.has_root_name())) {
160166
std::string msg = std::string("MCB: ") + kind + " '" + rawPath +
161167
"' is an absolute path outside the document root; rejected "
162168
"under the untrusted-content load policy";

0 commit comments

Comments
 (0)