ITADN

POST /download/info includes locked folder assets, leaking their IDs and sizes and breaking the follow-up archive request

#30847ClosedDjodyKort 创建于 10 天前
D
DjodyKortcommented
`POST /download/info` with `userId` set returns a download plan that includes locked folder assets, even when the session has not unlocked them. Two things follow from that: the plan leaks locked asset IDs and file sizes, and feeding that plan to `POST /download/archive` fails the whole request. ## What I found The `userId` branch of `getDownloadInfo` authorizes with `Permission.TimelineDownload` (`server/src/services/download.service.ts:26-30`): ```ts } else if (dto.userId) { const userId = dto.userId; await this.requireAccess({ auth, permission: Permission.TimelineDownload, ids: [userId] }); assets = this.downloadRepository.downloadUserId(userId); } ``` That permission compares user IDs and nothing else, so elevation never comes into it (`server/src/utils/access.ts:271-273`): ```ts case Permission.TimelineDownload: { return ids.has(auth.user.id) ? new Set([auth.user.id]) : new Set(); } ``` `Permission.AssetDownload`, a few cases above it in the same switch, does pass elevation into the ownership check (`server/src/utils/access.ts:136-137`). The query only excludes `Hidden` (`server/src/repositories/download.repository.ts:34-39`): ```ts downloadUserId(userId: string) { return builder(this.db) .where('asset.ownerId', '=', userId) .where('asset.visibility', '!=', AssetVisibility.Hidden) .stream(); } ``` so locked assets land in the plan together with their IDs and `fileSizeInByte`. `POST /download/archive` then authorizes with `Permission.AssetDownload`, which goes through `checkOwnerAccess` (`server/src/repositories/access.repository.ts:187-198`) and drops locked assets when the session is not elevated: ```ts .$if(!hasElevatedPermission, (eb) => eb.where('asset.visibility', '!=', AssetVisibility.Locked)) ``` and `requireAccess` throws as soon as one requested ID is missing from the allowed set (`server/src/utils/access.ts:37-40`). A single locked ID in the plan therefore fails the entire archive request, including the assets the session is allowed to download. ## Steps to reproduce 1. Use an account that has at least one locked folder asset alongside normal assets. 2. With a session that has not passed the PIN or biometric check, call `POST /download/info` with `{"userId": "<your own user id>"}`. 3. The returned `archives[].assetIds` contain the locked asset IDs, and the sizes count them too. 4. Call `POST /download/archive` with those asset IDs from the same session. It fails with `400 Not found or no AssetDownload access`, for the whole request rather than just the locked part. Expected: the plan leaves locked assets out unless the session has `hasElevatedPermission`, and the archive call then succeeds for the rest. ## Version Server v3.1.0-161, commit `9ab12b0c4`, self hosted Docker. ## Notes I found this by reading the code rather than from an incident, so the steps follow the code paths instead of a log. As far as I can tell no first party client (web, mobile or CLI) currently calls `/download/info` with `userId`, so this is reachable through the API rather than through the UI. The `assetIds` and `albumId` paths look fine, since they go through `Permission.AssetDownload` and `Permission.AlbumDownload`, which both do the elevation aware check.
关闭于 10 天前 1 条评论