From fd10b9723d66c77e9631c69b7bbcfd9f21b8de64 Mon Sep 17 00:00:00 2001
From: Ravi Khadiwala <ravi@signal.org>
Date: Wed, 14 Aug 2024 16:48:13 -0500
Subject: [PATCH] Add source length validation on backup media copy

---
 .../textsecuregcm/backup/BackupManager.java   |  2 +-
 .../controllers/ArchiveController.java        |  4 ++-
 .../controllers/ArchiveControllerTest.java    | 33 +++++++++++++++++++
 3 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/backup/BackupManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/backup/BackupManager.java
index 9aa6027f5..7ab32ea26 100644
--- a/service/src/main/java/org/whispersystems/textsecuregcm/backup/BackupManager.java
+++ b/service/src/main/java/org/whispersystems/textsecuregcm/backup/BackupManager.java
@@ -273,7 +273,7 @@ public class BackupManager {
       final List<CopyParameters> toCopy) {
     final long totalBytesAdded = toCopy.stream()
         .mapToLong(copyParameters -> {
-          if (copyParameters.sourceLength() > MAX_MEDIA_OBJECT_SIZE) {
+          if (copyParameters.sourceLength() > MAX_MEDIA_OBJECT_SIZE || copyParameters.sourceLength() < 0) {
             throw Status.INVALID_ARGUMENT
                 .withDescription("Invalid sourceObject size")
                 .asRuntimeException();
diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/ArchiveController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/ArchiveController.java
index 4820b6e2c..99e1a7e5c 100644
--- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/ArchiveController.java
+++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/ArchiveController.java
@@ -34,6 +34,7 @@ import javax.validation.constraints.Max;
 import javax.validation.constraints.Min;
 import javax.validation.constraints.NotBlank;
 import javax.validation.constraints.NotNull;
+import javax.validation.constraints.PositiveOrZero;
 import javax.validation.constraints.Size;
 import javax.ws.rs.BadRequestException;
 import javax.ws.rs.ClientErrorException;
@@ -479,6 +480,7 @@ public class ArchiveController {
 
       @Schema(description = "The length of the source attachment before the encryption applied by the copy operation")
       @NotNull
+      @PositiveOrZero
       int objectLength,
 
       @Schema(description = "mediaId to copy on to the backup CDN, encoded in URL-safe padded base64", implementation = String.class)
@@ -575,7 +577,7 @@ public class ArchiveController {
       @Schema(description = "A list of media objects to copy from the attachments CDN to the backup CDN")
       @NotNull
       @Size(min = 1, max = 1000)
-      List<CopyMediaRequest> items) {}
+      List<@Valid CopyMediaRequest> items) {}
 
   public record CopyMediaBatchResponse(
 
diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/ArchiveControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/ArchiveControllerTest.java
index d427e1c72..21de15c31 100644
--- a/service/src/test/java/org/whispersystems/textsecuregcm/controllers/ArchiveControllerTest.java
+++ b/service/src/test/java/org/whispersystems/textsecuregcm/controllers/ArchiveControllerTest.java
@@ -443,6 +443,39 @@ public class ArchiveControllerTest {
     assertThat(r4.failureReason()).isNotBlank();
   }
 
+
+  @Test
+  public void copyMediaWithNegativeLength() throws VerificationFailedException {
+    final BackupAuthCredentialPresentation presentation = backupAuthTestUtil.getPresentation(
+        BackupLevel.MEDIA, backupKey, aci);
+    when(backupManager.authenticateBackupUser(any(), any()))
+        .thenReturn(CompletableFuture.completedFuture(backupUser(presentation.getBackupId(), BackupLevel.MEDIA)));
+    final byte[][] mediaIds = new byte[][]{TestRandomUtil.nextBytes(15), TestRandomUtil.nextBytes(15)};
+    final Response r = resources.getJerseyTest()
+        .target("v1/archives/media/batch")
+        .request()
+        .header("X-Signal-ZK-Auth", Base64.getEncoder().encodeToString(presentation.serialize()))
+        .header("X-Signal-ZK-Auth-Signature", "aaa")
+        .put(Entity.json(new ArchiveController.CopyMediaBatchRequest(List.of(
+            new ArchiveController.CopyMediaRequest(
+                new ArchiveController.RemoteAttachment(3, "abc"),
+                1,
+                mediaIds[0],
+                TestRandomUtil.nextBytes(32),
+                TestRandomUtil.nextBytes(32),
+                TestRandomUtil.nextBytes(16)),
+
+            new ArchiveController.CopyMediaRequest(
+                new ArchiveController.RemoteAttachment(3, "def"),
+                -1,
+                mediaIds[1],
+                TestRandomUtil.nextBytes(32),
+                TestRandomUtil.nextBytes(32),
+                TestRandomUtil.nextBytes(16))
+        ))));
+    assertThat(r.getStatus()).isEqualTo(422);
+  }
+
   @CartesianTest
   public void list(
       @CartesianTest.Values(booleans = {true, false}) final boolean cursorProvided,