Vulnerability GHSA-5v7c-xpfp-p65m

Reporter: Jagadesh Achanta

Target: 0xJacky/nginx-ui

Affected version / commit tested:

• App version: 2.3.10
• Commit: 4948fba8744e82c3d7003f68ee164e9f49421066
• Disclosure channel: GitHub Private Vulnerability Reporting
• Credential reference: cred:github-jagstack-gh-cli

AI assistance disclosure

This report was prepared by Jagadesh Achanta with assistance from OpenAI/Codex for public source review, local evidence collection, and report drafting.

Summary

Nginx UI has a short-lived OTP/passkey "secure session" step-up boundary for users with 2FA enabled. RequireSecureSession() rejects an OTP-enabled user's request unless the request includes a valid X-Secure-Session-ID issued after a fresh OTP/passkey verification.

The project already applies this step-up middleware to sensitive operations such as:

• Nginx config create/edit/rename/delete;
• site save/delete/enable/disable/rename/duplicate;
• stream save/delete/enable/disable/rename/duplicate;
• settings save and protected secret reads;
• backup restore.

However, several similarly sensitive authenticated mutation routes remain outside RequireSecureSession(). An attacker who obtains an authenticated JWT/session for a 2FA-enabled user, but does not have the victim's fresh OTP/passkey secure-session token, can still perform those operations.

The affected route groups include:

• automatic backup create/modify/delete/restore/test-S3 operations;
• DNS credential create/edit/delete operations;
• certificate and ACME user create/modify/delete/register operations;
• DNS domain, record, and DDNS create/update/delete operations;
• Nginx reload/restart, stub-status toggle, performance-setting update, and module-cache refresh operations;
• site/stream advanced-mode toggles and batch updates;
• auto-certificate binding/unbinding for sites;
• site navigation order and health-check updates.

This report is distinct from my earlier Nginx UI report about auto-backup path traversal. This report is about missing 2FA step-up authorization on sensitive authenticated mutation routes.

Why this is a security boundary

internal/middleware/secure_session.go only requires a secure-session token for users with OTP enabled. For OTP users, it reads X-Secure-Session-ID from the header or query string and aborts with 401 Unauthorized if the secure session is missing or invalid:

if !cUser.EnabledOTP() {
    c.Next()
    return
}
ssid := c.GetHeader("X-Secure-Session-ID")
if ssid == "" {
    ssid = c.Query("X-Secure-Session-ID")
}
if ssid == "" {
    c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{
        "message": "Secure Session ID is empty",
    })
    return
}

if user.VerifySecureSessionID(ssid, cUser.ID) {
    c.Next()
    return
}

internal/user/otp.go issues the secure-session ID for only 10 minutes:

func SetSecureSessionID(userId uint64) (sessionId string) {
    sessionId = uuid.NewString()
    cache.Set(secureSessionIDCacheKey(sessionId), userId, 10*time.Minute)
    return
}

The UI/server already rely on this boundary for sensitive actions. For example, api/config/router.go wraps config mutations with RequireSecureSession():

o := r.Group("", middleware.RequireSecureSession())
{
    o.POST("configs", AddConfig)
    o.POST("config", EditConfig)
    o.POST("config_mkdir", Mkdir)
    o.POST("config_rename", Rename)
    o.POST("config_delete", DeleteConfig)
}

api/sites/router.go protects normal site mutations:

o := r.Group("", middleware.RequireSecureSession())
{
    o.POST("sites/:name/rename", RenameSite)
    o.POST("sites/:name/enable", EnableSite)
    o.POST("sites/:name/disable", DisableSite)
    o.POST("sites/:name", SaveSite)
    o.DELETE("sites/:name", DeleteSite)
    o.POST("sites/:name/duplicate", DuplicateSite)
    o.POST("sites/:name/maintenance", EnableMaintenanceSite)
}

api/streams/router.go protects normal stream mutations, and api/settings/router.go protects secret reads/settings saves:

o := r.Group("", middleware.RequireSecureSession())
{
    o.POST("streams/:name", SaveStream)
    o.POST("streams/:name/rename", RenameStream)
    o.POST("streams/:name/enable", EnableStream)
    o.POST("streams/:name/disable", DisableStream)
    o.DELETE("streams/:name", DeleteStream)
    o.POST("streams/:name/duplicate", Duplicate)
}

r.GET("settings/protected", middleware.RequireSecureSession(), GetProtectedSetting)
r.POST("settings", middleware.RequireSecureSession(), SaveSettings)

The repository's own tests also verify this expected behavior. api/config/security_test.go, api/sites/security_test.go, and api/streams/security_test.go each assert that OTP-enabled users receive 401 Unauthorized when protected mutations are attempted without a secure session.

Affected routes outside the step-up boundary

The affected routes are under the authenticated private router:

g := root.Group("/", middleware.AuthRequired(), middleware.Proxy())

but they do not add middleware.RequireSecureSession().

Automatic backup management

api/backup/router.go protects manual restore:

r.POST("/restore", middleware.AuthRequired(), middleware.RequireSecureSession(), middleware.EncryptedForm(), RestoreBackup)

but the automatic backup management router has no secure-session middleware:

r.POST("/auto_backup", CreateAutoBackup)
r.POST("/auto_backup/:id", ModifyAutoBackup)
r.DELETE("/auto_backup/:id", DestroyAutoBackup)
r.PATCH("/auto_backup/:id", RestoreAutoBackup)
r.POST("/auto_backup/test_s3", TestS3Connection)

Those handlers create or modify backup jobs, register cron jobs, test attacker-supplied S3 settings, and store S3 credentials:

"s3_access_key_id":     "omitempty",
"s3_secret_access_key": "omitempty",
...
if ctx.Model.Enabled {
    if err := cron.AddAutoBackupJob(ctx.Model.ID, ctx.Model.CronExpression); err != nil {

The model stores S3 secret material:

S3AccessKeyID     string `json:"s3_access_key_id" gorm:"comment:S3 access key ID;serializer:json[aes]"`
S3SecretAccessKey string `json:"s3_secret_access_key" gorm:"comment:S3 secret access key;serializer:json[aes]"`

Certificate, ACME, and DNS credential management

api/certificate/router.go registers credential and certificate mutations without RequireSecureSession():

r.POST("dns_credentials", AddDnsCredential)
r.POST("dns_credentials/:id", EditDnsCredential)
r.DELETE("dns_credentials/:id", DeleteDnsCredential)

r.POST("certs", AddCert)
r.POST("certs/:id", ModifyCert)
r.DELETE("certs/:id", RemoveCert)
r.PUT("cert_sync", SyncCertificate)

r.POST("acme_users", CreateAcmeUser)
r.POST("acme_users/:id", ModifyAcmeUser)
r.POST("acme_users/:id/register", RegisterAcmeUser)
r.DELETE("acme_users/:id", DestroyAcmeUser)

The DNS credential handlers persist provider configuration:

dnsCredential := model.DnsCredential{
    Name:         json.Name,
    Config:       &json.Config,
    Provider:     json.Provider,
    ProviderCode: providerCode,
}
...
err := d.Create(&dnsCredential)

The model serializes that configuration with AES:

Config *dns.Config `json:"config,omitempty" gorm:"serializer:json[aes]"`

DNS domains, records, and DDNS

api/dns/router.go registers DNS domain, record, and DDNS mutations without secure-session middleware:

group.POST("/domains", CreateDomain)
group.POST("/domains/:id", UpdateDomain)
group.DELETE("/domains/:id", DeleteDomain)
group.POST("/domains/:id/records", CreateRecord)
group.PUT("/domains/:id/records/:record_id", UpdateRecord)
group.DELETE("/domains/:id/records/:record_id", DeleteRecord)
group.PUT("/domains/:id/ddns", UpdateDDNSConfig)
group.DELETE("/domains/:id/ddns", DeleteDDNSConfig)

Those handlers call the DNS provider service and modify DDNS jobs:

record, err := svc.CreateRecord(c.Request.Context(), domainID, toRecordInput(payload))
...
record, err := svc.UpdateRecord(c.Request.Context(), domainID, recordID, toRecordInput(payload))
...
if err := cron.AddOrUpdateDDNSJob(domainID, cfg.IntervalSeconds); err != nil {

Nginx control and configuration-side effects

api/nginx/router.go registers several server/Nginx mutation endpoints without RequireSecureSession():

r.POST("nginx/reload", Reload)
r.POST("nginx/restart", Restart)
r.POST("nginx/stub_status", ToggleStubStatus)
r.POST("nginx/performance", UpdatePerformanceSettings)
r.POST("nginx/modules/refresh", RefreshModulesCache)

The handlers perform control actions or persist Nginx-related changes:

func Reload(c *gin.Context) {
    nginx.Control(nginx.Reload).Resp(c)
}

func Restart(c *gin.Context) {
    c.JSON(http.StatusOK, gin.H{
        "message": "ok",
    })
    go nginx.Restart()
}

if json.Enable {
    err = performance.EnableStubStatus()
} else {
    err = performance.DisableStubStatus()
}
...
reloadOutput, err := nginx.Reload()

err := performance.UpdatePerfOpt(&perfOpt)

Site and stream advanced/batch mutations

api/sites/router.go leaves these site mutations before the secure-session group:

r.PUT("sites", BatchUpdateSites)
r.POST("sites/:name/advance", DomainEditByAdvancedMode)
r.POST("auto_cert/:name", AddDomainToAutoCert)
r.DELETE("auto_cert/:name", RemoveDomainFromAutoCert)
r.POST("site_navigation/order", UpdateSiteOrder)
r.POST("site_navigation/health_check/:id", UpdateHealthCheck)

DomainEditByAdvancedMode() persists the site's Advanced mode:

_, err = s.Where(s.Path.Eq(path)).Update(s.Advanced, json.Advanced)

AddDomainToAutoCert() writes certificate automation settings including DNS credential ID, domains, challenge method, and ACME user:

err = certModel.Updates(&model.Cert{
    Name:            name,
    Domains:         json.Domains,
    AutoCert:        model.AutoCertEnabled,
    DnsCredentialID: json.DnsCredentialID,
    ChallengeMethod: json.ChallengeMethod,
    KeyType:         helper.GetKeyType(json.KeyType),
    ACMEUserID:      json.ACMEUserID,
})

api/streams/router.go protects normal stream save/delete operations, but leaves batch update and advanced-mode outside:

r.PUT("streams", BatchUpdateStreams)
...
r.POST("streams/:name/advance", AdvancedEdit)

AdvancedEdit() persists the stream's Advanced mode:

_, err = s.Where(s.Path.Eq(path)).Update(s.Advanced, json.Advanced)

Safe local proof

I did not access any external Nginx UI deployment. The following proof only inspects the checked-out source tree and confirms that:

• RequireSecureSession() is a real OTP-gated 10-minute step-up mechanism;
• high-risk control routes are intentionally protected elsewhere;
• the affected route groups are authenticated but outside RequireSecureSession();
• the affected handlers perform persistent or server-control side effects.

Run:

python3 reports/nginx-ui-missing-secure-session-step-up-poc.py

Observed output:

{
  "all_affected_groups_present": true,
  "all_controls_present": true,
  "all_side_effects_present": true,
  "commit": "4948fba8744e82c3d7003f68ee164e9f49421066",
  "result": "VULNERABLE_SOURCE_CONFIRMED",
  "target": "0xJacky/nginx-ui"
}

The full local proof output is available in my workspace as tmp/nginx-ui-missing-secure-session-step-up-poc-output.txt.

HTTP reproduction outline

This is a local-only reproduction outline for an isolated Nginx UI test instance, consistent with the project's security policy:

1. Create a test user with OTP enabled.
2. Log in as that user and keep only the normal authenticated session/JWT.
3. Do not obtain or send a fresh X-Secure-Session-ID.
4. Verify the expected protected control behavior:

POST /api/config
Authorization: <otp-user-session>
Content-Type: application/json

{ "name": "example.conf", "content": "server { listen 80; }" }

Expected result: 401 Unauthorized because config edits require RequireSecureSession().

1. Attempt an affected operation with the same OTP-enabled session and no X-Secure-Session-ID, for example:

POST /api/dns_credentials
Authorization: <otp-user-session>
Content-Type: application/json

{
  "name": "attacker-controlled-test",
  "provider": "cloudflare",
  "provider_code": "cloudflare",
  "token": "test-token"
}

Expected secure behavior: 401 Unauthorized, matching the secure-session requirement for other sensitive configuration changes.

Actual behavior from source review: the request reaches AddDnsCredential() because the DNS credential router is under AuthRequired() but does not apply RequireSecureSession().

The same missing step-up applies to the other route groups listed above.

Impact

If an attacker obtains the authenticated web session/JWT for a 2FA-enabled Nginx UI user, the secure-session boundary should limit what they can do until they also pass a fresh OTP/passkey challenge. The current route map leaves major administrative actions outside that boundary.

Depending on the victim user's privileges and deployment configuration, this can allow the attacker to:

• add, edit, or remove DNS provider credentials;
• alter certificate and ACME automation;
• create, modify, delete, or restore automatic backup jobs;
• configure S3 backup destinations and trigger S3 connection tests;
• alter DNS domains, records, or DDNS schedules through configured providers;
• restart or reload Nginx;
• enable/disable stub-status and change performance settings;
• switch sites/streams into advanced mode or change site automation bindings.

This undermines the value of the secure-session step-up for OTP-enabled accounts. I am not claiming pre-authentication access or remote code execution from this issue. The issue is missing step-up authorization for sensitive authenticated mutations.

Suggested fix

Recommended mitigation:

1. Apply middleware.RequireSecureSession() to all high-impact authenticated mutation routes that alter server, Nginx, certificate, DNS, backup, credential, or automation state.
2. At minimum, protect:
   • auto-backup create/modify/delete/restore/test-S3;
   • DNS credential create/edit/delete;
   • certificate and ACME user mutations;
   • DNS domain, record, and DDNS mutations;
   • Nginx reload/restart/stub-status/performance/module-cache mutation routes;
   • site/stream advanced-mode, batch, auto-cert, navigation order, and health-check mutations.
3. Add regression tests mirroring the existing config/site/stream security tests:
   • OTP-enabled user without X-Secure-Session-ID receives 401;
   • OTP-enabled user with a valid secure-session ID succeeds;
   • non-OTP users retain existing behavior if that is the intended product policy.
4. Consider centralizing "sensitive mutation" routing to make future omissions harder.