<#843 security: fix 13 findings from security audi...
# osctrl
g
#843 security: fix 13 findings from security audit Pull request opened by alvarofraguas Summary Addresses the 13 security findings identified in the evilsocket audit report. All fixes have been deployed and verified on a dockerized test environment (osctrl-api + osctrl-admin + osctrl-tls + Keycloak + Auth0 SAML) with 24 automated integration tests passing. Findings fixed | # | Severity | Finding | Fix | | --- | -------- | ------------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------- | | 1 | High | IDOR — delete node in env A by UUID belonging to env B | GetByUUIDEnv lookup before delete in nodes.go | | 2 | High | Empty-password login for SSO-provisioned users | Reject empty passwords with timing-safe dummy bcrypt; hash random token for JIT users instead of empty string | | 3 | High | Federated login overwrites local admin account | Guard in resolveFederatedUser: reject when AuthSource=="" (local); allow cross-protocol switch with ChangeAuthSource() | | 4 | High | Same local-account hijack in legacy admin OIDC flow | Mirror guard in cmd/admin/oidc.go resolveOIDCUser | | 5 | Medium | OIDC email claim trusted without email_verified | Gate on email_verified in claims.go; reject unverified emails | | 6 | High | CarveBlock accepted for wrong environment | carve.EnvironmentID != env.ID check in post.go | | 7 | High | Path traversal in carved file reassembly | filepath.Base() on every path segment in carves/utils.go | | 8 | High | Log ingestion cross-env — node in env A logs to env B | Hard return on envid != node.EnvironmentID in logging/process.go | | 9 | Medium | Environment detail leaks secrets to non-admin users | projectEnvironmentView() strips Secret, Certificate, Flags, ConfigTLS for non-admin | | 10 | Medium | XSS in admin node detail template | jQuery .text() instead of .html() for user-controlled fields | | 11 | Medium | Carve permission check uses wrong constant | CarveLevel throughout carves.go handlers | | 12 | Medium | Cross-env carve archive download | Env ownership check before serving archive in get.go | | 13a | Medium | S3 carve block_id unbounded — write to arbitrary keys | Bounds check [0, 9998] in s3.go | | 13b | Medium | TLS secret comparison not timing-safe | subtle.ConstantTimeCompare in post.go | Additional changesLogin: Removed per-environment login requirement —
POST /api/v1/login
authenticates against all environments. Simplifies the SPA login flow (no env dropdown needed pre-auth). • SAML cross-protocol: Users who were JIT-provisioned via OIDC can now log in via SAML (and vice versa) with
auth_source
updated on each federated login. • Logout protocol detection: Logout handler now uses the
osctrl_id_token
cookie (set only by OIDC callback) to determine which IdP logout flow to trigger, instead of relying on the user's creation-time
auth_source
. Password users no longer get incorrectly redirected to the SAML IdP logout URL. Test plan • 24 automated integration tests on dockerized environment (Keycloak OIDC + Auth0 SAML + password auth) • Verified IDOR: delete node in env B from env A returns 404 • Verified empty-password rejection for SSO users (timing-safe) • Verified local-account hijack blocked for both OIDC and SAML • Verified cross-env carve block / log ingestion / archive download blocked • Verified env detail response stripped for non-admin • Verified XSS escape in admin node template • Verified S3 block_id bounds and timing-safe secret comparison • Verified logout protocol detection: OIDC → end_session_endpoint, SAML → /v2/logout with returnTo, password → /login •
go vet ./...
clean • Frontend builds clean (
npm run build
) jmpsec/osctrl