GHSA-pxf8-6wqm-r6hh: Note Mark: OIDC-registered users authenticated by submitting password "null"
Summary`IsPasswordMatch` in `backend/db/models.go` falls back to a hard-coded `bcrypt("null")` placeholder whenever a user has no stored password. OIDC-registered users are created with an empty password, so anyone who submits `password: "null"` to the internal login endpoint receives a valid session for that user. The bypass is unauthenticated and requires no user interaction.Details`backend/db/models.go:36` defines the placeholder hash used by the timing-attack mitigation inside `IsPasswordMatch`:`IsPasswordMatch` (`backend/db/models.go:46-58`) substitutes that placeholder when the stored password is empty:OIDC-registered users are stored with an empty password at `backend/services/auth.go:102-115`:The internal login endpoint (`POST /api/auth/token`, handled at `backend/services/auth.go:20-54`) calls `IsPasswordMatch` with the caller-supplied password. For any OIDC-only user, `bcrypt.CompareHashAndPassword(nullPasswordHash, []byte("null"))` returns nil, the function returns true, and the server issues a `Auth-Session-Token` cookie.`EnableInternalLogin` defaults to `true`, and `GET /api/info` discloses both OIDC configuration and internal-login status. `enableAnonymousUserSearch` also defaults to `true`, so an unauthenticated caller enumerates usernames via `GET /api/users/search` before touching the login endpoint.Once the session is issued, `PUT /api/users/me/password` accepts `existingPassword: "null"` because the same `IsPasswordMatch` routine verifies the existing password. The caller writes a new password onto the OIDC user's row, which locks the legitimate OIDC user out on the next internal-login path.Proof of ConceptTested against `note-mark` v0.19.2.Step 1: Start note-mark pointed at any OIDC provider and set `OIDC__ENABLE_USER_CREATION=true`. The defaults for `ENABLE_INTERNAL_LOGIN` and `ENABLE_ANONYMOUS_USER_SEARCH` do not need to be changed.Step 2: Alice registers via the OIDC flow. `TryCreateNewOidcUser` stores her row with `Password = []byte("")`.Step 3: Bob confirms the preconditions.Step 4: Bob logs in as Alice via the internal endpoint.Response:Step 5: Bob uses the cookie to read Alice's account.Step 6: Bob persists access by writing his own password onto Alice's row.Alice's next internal-login attempt fails; her OIDC flow still works, but Bob now holds a second valid credential on the same row.A companion script that drives all six steps ships at `pocs/poc_014_null_password_bypass.sh`.ImpactEvery OIDC-only user on a note-mark deployment with `ENABLE_INTERNAL_LOGIN=true` (the default) is one HTTP request from takeover. Bob reads Alice's private notebooks, her note markdown, and her uploaded assets. He writes, edits, or deletes anything Alice owns. Step 6 grants persistent access and costs Alice her account until the maintainer clears the row by hand.The default configuration ships both authentication paths side by side, so any site that turns on OIDC is affected without further misconfiguration on the operator's part.Recommended FixThe clearest fix rejects the login path for rows with no stored password. Add the check after the user lookup in `GetAccessToken`:The equivalent change belongs in `UpdateUserPassword` at `backend/services/users.go:53-61`, since the same routine verifies `existingPassword` during the persistence step.Replacing `nullPasswordHash` with a per-instance unguessable plaintext closes the hole too, but relies on the placeholder staying secret:The explicit empty-password check is preferable because the intent is readable in the source.*Found by [aisafe.io](https://aisafe.io)*
CVSS v3.1: 9.4