Summary
PrivateApi.Notifications (dotnet/EcencyApi/Handlers/PrivateApi.UserData1.cs:14-62) lets a value in the request body decide whose notifications are returned, instead of deriving that solely from the validated auth code. The result is that the endpoint does not actually authenticate the reader.
Found while tracing the notification delivery path for the follow outage (ecency/enotify-py#20). Not related to that fix, and not a regression from the dotnet port: the header comment says this is a port of src/server/handlers/private-api.ts lines 1004-1257, so the same shape exists upstream and should be checked there too.
The logic
varusername=awaitValidateCode(body);varuser=body.Field("user");if(string.IsNullOrEmpty(username)){if(!JsJson.IsTruthy(user)){awaitctx.SendText(401,"Unauthorized");return;}username=UserData1Helpers.Template(user);// (1)}// if user defined but not same as user's codeif(JsJson.IsTruthy(user)){username=UserData1Helpers.Template(user);// (2)}Two separate problems:
- No code required. When
ValidateCode yields nothing, a present user field satisfies the guard and becomes the account queried. The 401 is only reachable when user is absent, so supplying it is enough to pass. - A valid code is overridden anyway. Block (2) runs unconditionally, so even a correctly authenticated caller has their identity replaced by whatever the body says. The comment above it describes an intent ("if user defined but not same as user's code") that the code never implements: there is no comparison.
UnreadNotifications immediately below, at :81-92, is the correct pattern for the same data:
varusername=awaitValidateCode(body);if(string.IsNullOrEmpty(username)){awaitctx.SendText(401,"Unauthorized");return;}Second issue in the same handler
username, filter, since and limit are interpolated into the upstream path (:38-58) through Template(), which is a JS string-coercion emulator (:396-416) and performs no URL encoding. Values containing path or query separators therefore reach the upstream request as structure rather than as data, so a caller can influence which upstream endpoint is hit, not just its arguments.
The practical consequence is that the api-proxy nginx per-path allowlist is currently acting as a security control rather than as routing hygiene. It is the thing bounding which upstream endpoints are reachable this way, and it was not designed for that job.
Suggested fix
- Derive
username from ValidateCode only. If the endpoint genuinely needs to serve another account, compare explicitly and reject a mismatch rather than overwriting, which is what the existing comment already implies was intended. - Percent-encode every interpolated segment, or build the upstream URI from typed components so a value cannot contribute path or query structure.
- Apply the same review to the TypeScript original this was ported from, and audit the rest of
PrivateApi.UserData1.cs for other handlers reading an identity from the body. Notifications is the only one with the override pattern today, but the encoding issue is worth checking more broadly. - Worth a regression test asserting that a request whose body names a different account than its code either 401s or serves the code's account.
Scope
Read-only exposure of notification activity for an arbitrary named account. No write path, no credential disclosure. Deliberately not including a reproduction request here since the repository is public.
Summary
PrivateApi.Notifications(dotnet/EcencyApi/Handlers/PrivateApi.UserData1.cs:14-62) lets a value in the request body decide whose notifications are returned, instead of deriving that solely from the validated auth code. The result is that the endpoint does not actually authenticate the reader.Found while tracing the notification delivery path for the follow outage (ecency/enotify-py#20). Not related to that fix, and not a regression from the dotnet port: the header comment says this is a port of
src/server/handlers/private-api.tslines 1004-1257, so the same shape exists upstream and should be checked there too.The logic
Two separate problems:
ValidateCodeyields nothing, a presentuserfield satisfies the guard and becomes the account queried. The 401 is only reachable whenuseris absent, so supplying it is enough to pass.UnreadNotificationsimmediately below, at:81-92, is the correct pattern for the same data:Second issue in the same handler
username,filter,sinceandlimitare interpolated into the upstream path (:38-58) throughTemplate(), which is a JS string-coercion emulator (:396-416) and performs no URL encoding. Values containing path or query separators therefore reach the upstream request as structure rather than as data, so a caller can influence which upstream endpoint is hit, not just its arguments.The practical consequence is that the api-proxy nginx per-path allowlist is currently acting as a security control rather than as routing hygiene. It is the thing bounding which upstream endpoints are reachable this way, and it was not designed for that job.
Suggested fix
usernamefromValidateCodeonly. If the endpoint genuinely needs to serve another account, compare explicitly and reject a mismatch rather than overwriting, which is what the existing comment already implies was intended.PrivateApi.UserData1.csfor other handlers reading an identity from the body.Notificationsis the only one with the override pattern today, but the encoding issue is worth checking more broadly.Scope
Read-only exposure of notification activity for an arbitrary named account. No write path, no credential disclosure. Deliberately not including a reproduction request here since the repository is public.