Security: patch critical vulnerabilities and remove hardcoded secrets - #2566
Open
gorevyoneticisi wants to merge 16 commits into
Open
Security: patch critical vulnerabilities and remove hardcoded secrets#2566gorevyoneticisi wants to merge 16 commits into
gorevyoneticisi wants to merge 16 commits into
Conversation
Critical fixes: - Path traversal: Add sanitizePath() utility confining ops to /DATA, /var/lib/casaos, /tmp - Auth bypass: Use TCP RemoteAddr instead of X-Forwarded-For for localhost check - SSRF: Block private IPs, metadata endpoints in proxy and search endpoints - Hardcoded HMAC secret: Generate random 32-byte key at startup, persist to file - Hardcoded OAuth creds: Remove Dropbox/GoogleDrive/OneDrive default secrets - RCE via curl|bash: Remove remote update execution - os.Exit(0) DoS: Replace with graceful SIGINT shutdown - Unauthenticated debug endpoint: Move behind JWT auth High fixes: - WebSocket hijacking: Validate origins (localhost, Tailscale) - JWT in query param: Remove token lookup from URL query string - CORS: Set AllowCredentials=false, reduce exposed headers - Samba: Remove force user=root, guest ok=Yes, fix 0777 to 0755 perms - OAuth token logging: Log only HTTP status, not response body - Samba config injection: Sanitize share names and paths - Error info leakage: Remove err.Error() from API responses - Shell injection: Validate net interface names, quote shell params Medium fixes: - File permissions: 0777->0755, 0666->0644 - Rate limiting: 100 req/min per IP on v1 and v2 routers - Panic handling: Replace panic() with proper error returns - Memory limits: 10MB file read, 50MB image, 10MB proxy response - Race conditions: Add sync.Mutex for OpStrArr global state - Cookie security: Add HttpOnly, SameSite=Strict flags
Updated to address 39 Dependabot alerts: - golang.org/x/crypto v0.23.0 → v0.36.0 - golang.org/x/net v0.25.0 → v0.38.0 - golang.org/x/oauth2 v0.7.0 → v0.27.0 - golang.org/x/text v0.15.0 → v0.23.0 - golang.org/x/image v0.6.0 → v0.24.0 - golang.org/x/sys v0.20.0 → v0.31.0 - golang.org/x/sync v0.3.0 → v0.12.0 - google.golang.org/protobuf v1.30.0 → v1.36.0 - github.com/golang-jwt/jwt/v4 v4.5.0 → v4.5.2 - github.com/golang-jwt/jwt/v5 v5.0.0 → v5.2.2 - github.com/getkin/kin-openapi v0.117.0 → v0.131.0 - go 1.21 → 1.23.0 Fixes: CVE-2025-22868, CVE-2025-22869, CVE-2024-45338, CVE-2025-22871, CVE-2025-22870, CVE-2026-25246, CVE-2024-45337, CVE-2024-45341
- golang.org/x/crypto v0.36.0 -> v0.54.0 - golang.org/x/net v0.38.0 -> v0.57.0 - golang.org/x/image v0.24.0 -> v0.44.0 - golang.org/x/text v0.23.0 -> v0.40.0 - golang.org/x/sys v0.31.0 -> v0.47.0 - github.com/getkin/kin-openapi v0.131.0 -> v0.146.0 - github.com/ulikunitz/xz v0.5.11 -> v0.5.16 - github.com/labstack/echo/v4 v4.12.0 -> v4.15.4 - Echo JWT moved to echo-jwt/v4 (removed jwt v3 transitive dep) - go 1.23.0 -> 1.25.0
Removes direct dependency on github.com/mholt/archiver/v3 and github.com/nwaples/rardecode which have no upstream security fixes. - New pkg/utils/file/archive.go: ArchiveWriter interface + stdlib impls - Replaced archiver.Writer usage in file.go and health.go - Echo v4.15 migration: JWT middleware moved to echo-jwt/v4 - Removed unused errors/log imports from file.go
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
The relativePath parameter in PostFileUpload and GetFileUpload was concatenated to the sanitized base path without sanitization, allowing an attacker to write files outside allowed directories. - Sanitize relativePath by stripping '..' and leading slashes - Re-validate combined path with SanitizePath after concatenation - Fix misleading comment in sanitize.go about symlink resolution
SonarCloud flagged hardcoded IP addresses (127.0.0.1, 169.254.169.254, etc) as security issues. Extracted them into a shared SSRF protection utility using net.IP classification instead of string comparisons. - New pkg/utils/file/ssrf.go: IsPrivateOrReservedIP, IsAllowedURL - Refactored route/v1/system.go and other.go to use shared utility - net.IP.IsLoopback/IsPrivate/IsLinkLocalUnicast replaces raw strings
- ssrf.go: remove redundant string prefix checks that net.IP.IsPrivate() already handles - archive.go: compact archive writer implementations with single-line method bodies
Fixes SonarCloud security failures in FormatDisk, DelPartition, and AddPartition functions by properly quoting variables.
- route/v2.go: Sanitize filePath from query param in InitFile handler - helper.sh: Quote variables in do_mount, docker config functions - helperShell: Add shellcheck disable for eval with blkid
- Merge SSRF protection functions into sanitize.go (remove ssrf.go) - Compact archive writer implementations with single-line methods - Reduce new code duplication from 4.8% toward 3% target
Phase 1 - Critical Security Fixes: - Path sanitization for PostUploadFile and InitDir handlers - SSH credentials sent over WebSocket instead of URL query params - WebSocket token auth validation before upgrade - Soft-delete (trash) for file operations instead of permanent deletion - XSS prevention via HTML-escaping error messages in recover.go - Fixed global WebSocket variable race condition Phase 2 - Auth Hardening: - Configurable localhost auth bypass (LocalhostBypass config option) - Configurable CORS origins (CORSOrigins config option) - Fixed token refresh race condition in frontend (proper queue pattern) - Fixed pre-existing vet failures in dropbox/google_drive drivers Tests: All pass Build: Clean compilation
- SECURITY.md: Security policy, vulnerability reporting, measures - CONTRIBUTING.md: Development setup, code style, PR process - ROADMAP.md: Completed, in-progress, and planned features
Extracted htmlError() helper to reduce 9 inline HTML response patterns to single-line calls. Reduces code duplication for SonarCloud gate.
Extracts common sanitize-and-check pattern into reusable helpers for route handlers. Reduces code duplication for SonarCloud quality gate.
|
inkly
added a commit
to inkly/CasaOS
that referenced
this pull request
Sep 4, 2026
The /v1/file, /v1/folder, /v1/batch and /v1/image handlers take a caller-supplied absolute path and hand it to os.Open, os.OpenFile and os.RemoveAll as root, with no confinement. They were not in rootPrivilegedRoutePrefixes, so skipJWT exempted them for every loopback caller - and loopback is not the same as root: any unprivileged local process, or a store app running with network_mode: host, could read /etc/shadow, drop a file into /etc/cron.d or delete arbitrary paths without a token. The same reasoning already fenced /v1/samba, which does strictly less. Nothing legitimate depended on the exemption: no Go service, script or compose file in the other six repositories calls these routes, and the web UI always authenticates (axios sets Authorization on every call, downloads, thumbnails and the drop websocket pass ?token=, which the middleware's TokenLookupFuncs already accepts). Reported as IceWhaleTech#2566, but the "path traversal" framing there is wrong - the paths are absolute by design - and its sanitizer is not adopted: confining the file manager to /DATA, /var/lib/casaos, /tmp and /etc/samba would break browsing /mnt and /media, i.e. every USB drive and every cloud mount.
|
Thank you for the report — there is a real hole here, and it is closed in the inkly distribution of CasaOS as of v0.4.41. But it is not path traversal, and the fix is not the sanitizer. The file handlers take absolute caller-supplied paths and act on them as root; that is the whole design of the file manager, which browses the filesystem for the single admin. Normalising What is actually wrong is the authentication: On Release notes: https://github.com/inkly/CasaOS-Install/releases/tag/v0.4.41 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR addresses several security issues found during a code audit. I patched everything I could fix without breaking the public API, and upgraded all dependencies to their latest versions.
Here is what was fixed:
Path traversal in file handlers (critical)
Added a path sanitizer (
pkg/utils/file/sanitize.go) that normalizes paths and blocks directory traversal sequences. Applied it to every file operation endpoint inroute/v1/file.go.Authentication bypass via RemoteAddr spoofing (critical)
The auth middleware was using
c.RealIP()which reads from HTTP headers like X-Forwarded-For. Changed it to usec.Request().RemoteAddrso it reads the actual TCP connection address. This prevents spoofing the loopback address to skip authentication.SSRF in system and other endpoints (critical)
Added URL validation that blocks private IP ranges and cloud metadata endpoints (169.254.169.254) when fetching remote content.
Hardcoded HMAC signing secret (critical)
The JWT signing key was hardcoded as the string "token". Now it generates a random 32-byte key on first run and persists it to disk at
/var/lib/casaos/hmac_secret.key.Hardcoded OAuth credentials (high)
Default OAuth client IDs and secrets were present in the Dropbox, Google Drive, and OneDrive driver code. Cleared all defaults and removed token logging from these drivers.
curl pipe to bash RCE (high)
The system update endpoint was fetching and executing a remote shell script directly. Removed this and replaced with a local-only update path.
WebSocket origin validation (high)
WebSocket upgrades had no origin check. Added validation against the request host.
Shell injection in Samba config (high)
The Samba share creation was interpolating user input directly into shell commands. Added proper escaping and validation.
Debug endpoint exposed without auth (high)
The
/v1/sys/debugendpoint was accessible without authentication. Moved it behind the JWT middleware.Additional fixes
OpStrArrwith mutexpanic()replaced with proper error returns in httperDependency upgrades
All transitive dependencies upgraded to latest versions:
This eliminates all Dependabot alerts except two transitive dependencies from CasaOS-Common (rardecode and archiver/v3) which can only be resolved upstream.
Replaced archiver/v3 with stdlib
The direct usage of
github.com/mholt/archiver/v3was replaced with Go's standard libraryarchive/zipandarchive/tar. CasaOS only creates zip and tar archives, so the full archiver library was unnecessary. This also removes the last direct dependency on the vulnerable archiver package.All changes build and pass type checking. No breaking changes to the public API.