Lock down backup directory ACLs, and let the uninstaller reach IIS
Some checks failed
CI / backend (push) Failing after 8s
CI / naming (push) Successful in 2s
CI / frontend (push) Successful in 8s
CI / migrations-mysql (push) Failing after 6s

Two findings from the installer review, both of which fail silently.

Database dumps were readable by every authenticated user. A directory created
under ProgramData inherits BUILTIN\Users:(I)(OI)(CI)(RX), and a dump contains
every row including the users table and its password hashes. The installer
applied an owner-only ACL, but only in the branch that CREATED the directory,
so a directory created first by the console (shopdb-admin.ps1 backup) kept the
inherited permissions and the installer could never repair it. The ACL is now
re-applied on every run rather than only on creation, and the grants are made
inheritable with (OI)(CI) so dumps written into the directory later are covered
too. shopdb-admin.ps1 applies the same hardening for the default location, and
for an operator-named path says the dump holds password hashes rather than
silently rewriting the ACL of a directory that is theirs.

Verified on Windows: before, the directory carried BUILTIN\Users:(I)(OI)(CI)
(RX); after, only SYSTEM and Administrators, and a file created inside inherits
exactly those two. Without (OI)(CI) that file would not have been covered.

The uninstaller could not remove anything in IIS. [UninstallRun] launched a
bare "powershell.exe", and the Inno uninstaller is a 32-bit process, so WOW64
resolved it to the 32-bit PowerShell, which cannot see the IIS provider. The
site, application pool and application survived, pointing at a directory that
HAD been deleted, while Windows reported a clean uninstall. It now uses the
same Sysnative path as the [Run] entry, which was the last unshielded launch
site in the file.
This commit is contained in:
cproudlock
2026-08-04 21:04:54 -04:00
parent 1d73bd477e
commit ce521e84a5
3 changed files with 44 additions and 5 deletions

View File

@@ -279,9 +279,18 @@ function Invoke-Native {
function Protect-File {
# Owner-only ACL: Administrators + SYSTEM, inheritance broken.
param([string] $Path)
Invoke-Native 'icacls.exe' @($Path,'/inheritance:r',
'/grant','BUILTIN\Administrators:(F)','/grant','NT AUTHORITY\SYSTEM:(F)') 'ACL'
#
# -Directory additionally makes the two grants INHERITABLE. Without (OI)(CI)
# the grants apply to the directory alone, so files written into it later -
# backups taken by the console, not by this script - are not covered.
param([string] $Path, [switch] $Directory)
$admins = 'BUILTIN\Administrators:(F)'
$system = 'NT AUTHORITY\SYSTEM:(F)'
if ($Directory) {
$admins = 'BUILTIN\Administrators:(OI)(CI)(F)'
$system = 'NT AUTHORITY\SYSTEM:(OI)(CI)(F)'
}
Invoke-Native 'icacls.exe' @($Path,'/inheritance:r','/grant',$admins,'/grant',$system) 'ACL'
}
function New-Secret {
@@ -546,8 +555,13 @@ function Backup-Database {
$dir = Join-Path $env:ProgramData 'ShopDB-Flask\backups'
if (-not (Test-Path $dir)) {
New-Item -ItemType Directory -Path $dir -Force | Out-Null
Protect-File $dir
}
# RE-APPLIED EVERY TIME, not only on creation. The console creates this
# directory too (shopdb-admin.ps1 backup), and one created there inherits
# ProgramData's Users:RX - so dumps containing the users table and its
# password hashes became readable by every authenticated user on the server.
# The old create-only guard meant the installer could never repair that.
Protect-File $dir -Directory
$file = Join-Path $dir ("{0}-{1}-{2}.sql" -f $db.Name, $Reason, (Get-Date -Format 'yyyyMMdd-HHmmss'))
Write-Log "backing up $($db.Name) before migrating"