T09 · Insecure Skill Coding Practices
Warning
- Location
- scripts/deploy.ps1:360
- Finding
- Unrestricted VendorDir Allows Filesystem Writes Outside the Target Project## Vulnerability Details **File Location**: `scripts/deploy.ps1:74` and `scripts/deploy.ps1:360-370` **Vulnerability Type**: Path traversal resulting in deployment outside the intended directory **Risk Level**: Medium ### Vulnerable Code ```powershell [string]$VendorDir = 'tools\assist-skills', ``` ```powershell $vendorRoot = (Get-AbsolutePath (Join-Path $targetRoot $VendorDir)).TrimEnd('\', '/') if ([string]::Equals($vendorRoot, $skillRoot, $pathCompare)) { Write-Host " [skip] The vendor location is the skill package itself: $vendorRoot" } else { $copiedBefore = $script:copiedCount $skippedBefore = $script:skippedCount Copy-OneTree -SourceDir $skillRoot -DestinationDir $vendorRoot ` -ExcludeTopLevel @('_meta.json', '.clawhubignore', '.clawhub', '.git', 'node_modules') ``` ### Technical Analysis The `VendorDir` parameter is documented as a path relative to the target project, but the script does not enforce that requirement. It combines attacker- or user-controlled `$VendorDir` with `$targetRoot`, canonicalizes the result, and immediately uses the resulting path as the destination for recursively copying the Skill package. No validation confirms that the canonical `$vendorRoot` remains inside `$targetRoot`. A value containing parent-directory components, such as `..\outside`, therefore resolves outside the selected project. The script also does not explicitly reject rooted paths. The existing equality check only prevents vendoring directly onto `$skillRoot`. It does not protect the intended target-project boundary. ### Attack Path 1. An attacker, unsafe wrapper, or untrusted automation influences the arguments passed to the deployment script. 2. The script is invoked with vendoring enabled and a traversal path, for example: ```powershell .\deploy.cmd -Target C:\work\project -Vendor -VendorDir ..\outside ``` 3. `Join-Path` produces a path equivalent to `C:\work ...[truncated 910 chars]
- Remediation
- ## Remediation Suggestions Enforce that `VendorDir` is a relative path and that its canonical destination remains strictly beneath the target project: ```powershell if ([string]::IsNullOrWhiteSpace($VendorDir)) { throw 'VendorDir must not be empty.' } if ([System.IO.Path]::IsPathRooted($VendorDir)) { throw 'VendorDir must be relative to the target project.' } $vendorRoot = [System.IO.Path]::GetFullPath( (Join-Path $targetRoot $VendorDir) ).TrimEnd('\', '/') if (-not $vendorRoot.StartsWith($targetRoot + $sep, $pathCompare)) { throw 'VendorDir must remain inside the target project.' } ``` Additionally: 1. Reject a destination equal to the target root, preventing the package from being spread directly across the project. 2. Preserve the existing check that prevents copying onto the Skill source directory. 3. Before writing, resolve and validate existing parent directories to account for directory junctions or symbolic links that may redirect writes outside the project. 4. Add automated tests for `..\outside`, nested traversal, rooted paths, UNC paths, mixed separators, case variations, and valid nested paths. 5. Require explicit confirmation when `-Force` and `-Vendor` are used together, because that combination can overwrite files recursively.