CVE-2021-38621: Other (NVD-CWE-Other)
value: The remove API in v1/controller/cloudStorage/alibabaCloud/remove/index.ts in netless Agora Flat Server before 2021-07-30 mishandles file ownership.
CWE description: NVD is only using a subset of CWE for mapping instead of the entire CWE, and the weakness type is not covered by that subset.
CVSS v3 base score: 9.1
Published: 2021-08-13T15:15Z
Repository: https://github.com/netless-io/flat-server
Language: TypeScript
Files changed: src/v1/controller/cloudStorage/alibabaCloud/remove/index.ts
Commit date: 2021-07-30 18:04:48 +0800
Commit message: refactor(cloudStorage): improve file onwer check in remove api (#392)


Diff: diff --git a/src/v1/controller/cloudStorage/alibabaCloud/remove/index.ts b/src/v1/controller/cloudStorage/alibabaCloud/remove/index.ts
index 3581cf5..512b177 100644
--- a/src/v1/controller/cloudStorage/alibabaCloud/remove/index.ts
+++ b/src/v1/controller/cloudStorage/alibabaCloud/remove/index.ts
@@ -1,11 +1,11 @@
 import { createQueryBuilder, getConnection, In } from "typeorm";
 import { Region, Status } from "../../../../../constants/Project";
 import {
     CloudStorageConfigsDAO,
-    CloudStorageUserFilesDAO,
     CloudStorageFilesDAO,
+    CloudStorageUserFilesDAO,
 } from "../../../../../dao";
 import { CloudStorageFilesModel } from "../../../../../model/cloudStorage/CloudStorageFiles";
 import { CloudStorageUserFilesModel } from "../../../../../model/cloudStorage/CloudStorageUserFiles";
 import { FastifySchema, Response, ResponseError } from "../../../../../types/Server";
 import { getFilePath, ossClient } from "../Utils";
@@ -39,10 +39,12 @@ export class AlibabaCloudRemoveFile extends AbstractController<RequestType, Resp
 
     public async execute(): Promise<Response<ResponseType>> {
         const { fileUUIDs } = this.body;
         const userUUID = this.userUUID;
 
+        await this.assertFilesOwnerIsCurrentUser();
+
         const fileInfo: FileInfoType[] = await createQueryBuilder(CloudStorageUserFilesModel, "fc")
             .addSelect("f.file_uuid", "file_uuid")
             .addSelect("f.file_name", "file_name")
             .addSelect("f.file_size", "file_size")
             .addSelect("f.region", "region")
@@ -58,11 +60,14 @@ export class AlibabaCloudRemoveFile extends AbstractController<RequestType, Resp
                 },
             )
             .getRawMany();
 
         if (fileInfo.length === 0) {
-            throw new ControllerError(ErrorCode.FileNotFound);
+            return {
+                status: Status.Success,
+                data: {},
+            };
         }
 
         const cloudStorageConfigsInfo = await CloudStorageConfigsDAO().findOne(["total_usage"], {
             user_uuid: userUUID,
         });
@@ -136,10 +141,22 @@ export class AlibabaCloudRemoveFile extends AbstractController<RequestType, Resp
     }
 
     public errorHandler(error: Error): ResponseError {
         return this.autoHandlerError(error);
     }
+
+    private async assertFilesOwnerIsCurrentUser(): Promise<void> {
+        const filesOwner = await CloudStorageUserFilesDAO().find(["user_uuid"], {
+            file_uuid: In(this.body.fileUUIDs),
+        });
+
+        for (const { user_uuid } of filesOwner) {
+            if (user_uuid !== this.userUUID) {
+                throw new ControllerError(ErrorCode.NotPermission);
+            }
+        }
+    }
 }
 
 interface RequestType {
     body: {
         fileUUIDs: string[];

Vulnerable code:     CloudStorageUserFilesDAO,
            throw new ControllerError(ErrorCode.FileNotFound);
Fixed code:     CloudStorageUserFilesDAO,
        await this.assertFilesOwnerIsCurrentUser();

            return {
                status: Status.Success,
                data: {},
            };

    private async assertFilesOwnerIsCurrentUser(): Promise<void> {
        const filesOwner = await CloudStorageUserFilesDAO().find(["user_uuid"], {
            file_uuid: In(this.body.fileUUIDs),
        });

        for (const { user_uuid } of filesOwner) {
            if (user_uuid !== this.userUUID) {
                throw new ControllerError(ErrorCode.NotPermission);
            }
        }
    }