mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-11 18:00:54 -10:00
Vasanth T
91b413c514
* fix(sandbox): stop the Windows write jail honouring Everyone-granted paths The restricted token put the World SID (S-1-1-0, Everyone) in its restricted-SID list unconditionally, from the original sandbox baseline and with no comment saying why. A WRITE_RESTRICTED token runs two checks for a write and needs both: the normal one against its enabled SIDs, and a second against the restricted list. The write jail rests entirely on that second check only succeeding where Zero has explicitly ACL'd a capability SID. Everyone is a SID every principal carries, so on any path whose DACL grants Everyone write, the restricted half passed for free and confinement fell back to the ordinary user's own permissions -- the exact boundary this token exists to be stricter than. Reachable with no privilege, no symlink and no race: an Everyone-writable directory is enough, and share roots opened with Everyone:F and loose third-party installer ACLs supply them. Ruled out as the common case: C:\Users\Public\Documents grants BATCH, not Everyone. The runner already stated the rule this broke -- it declines to add the user SID, Administrators or SYSTEM because "each has write access nearly everywhere" -- while listing Everyone among the restricted SIDs. Removed only from the WRITE_RESTRICTED token, because without that flag it is load-bearing rather than gratuitous. The restricted-SID check then applies to reads too, and default Windows DACLs grant BUILTIN\Users rather than anything in this list, so a token with no Everyone cannot open cmd.exe and dies at launch with STATUS_ACCESS_DENIED (0xC0000022). Dropping it there was tried first and takes TestWindowsUnelevatedRealSandboxSmoke with it. That path is taken only when a profile configures DenyRead, which is already the posture trading capability for read-deny enforcement (#612), so the bypass survives there deliberately and narrowly; closing it needs a read-side grant that is not a universal group. Verified against real tokens rather than reasoned about. The new smoke test drives the real runner and asserts three outcomes that only agree when the jail is intact: a granted root is writable, an ordinary outside path is denied, and an Everyone-writable outside path is denied too. Before the change its third case wrote the file with exit 0; the first two were already correct, which is what made the gap invisible. Restoring the SID unconditionally turns it red again. * test(sandbox): make a denied write distinguishable from a runner failure The denial assertions expected exit 1, which the runner also returns for its own errors -- a failed marker validation, a bad argument, a token it could not build. Written that way the test passes both when the sandbox denies the write and when nothing ever ran, and the second case proves nothing while being indistinguishable from success. That is a poor property for any test and a bad one for this test, whose entire job is to notice a confinement regression. The leading "granted root is writable" assertion covered part of it, but said nothing about whether the two later commands launched. The denial commands now report a distinctive exit code the runner cannot produce for itself, so the code proves cmd.exe actually ran and its redirect was refused. Re-checked in both directions rather than assumed: the test passes against the fix, and restoring the World SID unconditionally fails it with "exit code = 0, want 77" -- still catching the bug it was written for. Raised by CodeRabbit on #865.