GHSA-h246-wpgf-vmq5: High severity go/github.com/0xJacky/Nginx-UI vulnerability

Summary

Incomplete fix of GHSA-5v7c-xpfp-p65m: the secure-session (OTP step-up) requirement added to the nginx, cert, dns, backup, site, and stream mutation routers was not applied to the parallel api/cluster router, so cluster node management and cluster-wide nginx reload/restart run with only a JWT and no step-up. An authenticated user whose JWT is stolen or persisted, but who does not hold a fresh secure-session, can perform cluster node CRUD (including reading and rewriting a node token secret) and trigger cluster-wide nginx reload/restart. Confirmed at HEAD 2cb7ee9102c9d87274de2fa104db804841d140a0.

The defect

GHSA-5v7c-xpfp-p65m required a fresh secure session (an OTP step-up beyond the JWT) for sensitive mutation routes. The fix wrapped the nginx, cert, dns, backup, site, and stream mutation handlers in middleware.RequireSecureSession() and added reload/restart to the MCP sensitive-tool list. It did not touch the physically separate api/cluster package, whose router registers the same class of sensitive operations on the bare authenticated group.

The fixed sibling, api/nginx/router.go:

go o := r.Group("", middleware.RequireSecureSession()) // line 28 { o.POST("nginx/reload", Reload) // line 30 o.POST("nginx/restart", Restart) // line 31 }

The missed router, api/cluster/router.go, registers every sensitive operation directly on r with no RequireSecureSession wrapper:

go nodeGroup := r.Group("nodes") // line 9, no step-up { nodeGroup.POST("", AddNode) // line 12 nodeGroup.POST("/:id", EditNode) // line 13 nodeGroup.DELETE("/:id", DeleteNode) // line 14 } r.POST("nodes/reloadnginx", ReloadNginx) // line 17 r.POST("nodes/restartnginx", RestartNginx) // line 18 r.POST("namespaces", AddNamespace) // line 22 r.POST("namespaces/:id", ModifyNamespace) // line 23 r.DELETE("namespaces/:id", DeleteNamespace) // line 24

Both routers mount on the same group in router/routers.go: g := root.Group("/", middleware.AuthRequired(), middleware.Proxy()) (line 87); nginx.InitRouter(g) creates its own RequireSecureSession subgroup, cluster.InitRouter(g) does not. So the cluster routes inherit only AuthRequired and Proxy, exactly the pre-fix posture the advisory closed for the nginx routes. AuthRequired has no role gate, so any authenticated user reaches them.

Attacker model and impact

An authenticated, OTP-enabled user who does not present a fresh X-Secure-Session-ID (the parent advisory's model: a stolen or persisted JWT used without the step-up). Such a user can: add/edit/delete cluster nodes (AddNode/EditNode store an AES-serialized node token, the secret used to control a remote node, so this reads back and rewrites a cross-node credential); trigger nodes/reloadnginx and nodes/restartnginx across the cluster (the exact reload/restart action class the fix protected on the single-node path); and add/modify/delete/reorder namespaces.

Proof of concept: with a valid JWT but no fresh secure session, call POST /api/nodes (AddNode) or POST /api/nodes/reloadnginx. The nginx equivalent POST /api/nginx/reload returns the secure-session challenge; the cluster route succeeds.

Verification

Source-verified at HEAD: the nginx router wraps reload/restart in RequireSecureSession, the cluster router registers node/namespace/reload/restart directly on the group with no such wrapper, and both mount on the AuthRequired+Proxy-only group. I did not stand up a live nginx-ui.

Suggested fix

Wrap the cluster router's mutation handlers (node CRUD, nodes/reloadnginx, nodes/restartnginx, namespace CRUD) in middleware.RequireSecureSession(), mirroring api/nginx/router.go. Secondary lower-confidence items worth checking in the same pass: the system/restart handler, the core-upgrade websocket, and the upstream socket PUT also appear to run without the step-up (I did not fully trace these).