fix: validate SOGS attachment sizes

ts/receiver/attachments.ts

@@ -13,6 +13,7 @@ import { OpenGroupRequestCommonType } from '../data/types';
 import { downloadFileFromFileServer } from '../session/apis/file_server_api/FileServerApi';
 import { FileFromFileServerDetails } from '../session/apis/file_server_api/types';
 import { MultiEncryptWrapperActions } from '../webworker/workers/browser/libsession_worker_interface';
+import { validateDownloadedSogsAttachment } from './sogsAttachmentDownload';
 
 /**
  * Note: the url must have the serverPubkey as a query parameter
@@ -105,9 +106,10 @@ export async function downloadAttachmentFs(attachment: {
 export async function downloadAttachmentSogsV3(
   attachment: {
     url: string;
-    size: number | null;
+    size?: number | null;
   },
-  roomInfos: OpenGroupRequestCommonType
+  roomInfos: OpenGroupRequestCommonType,
+  options: { allowUnknownSize?: boolean } = {}
 ) {
   const roomDetails = OpenGroupData.getV2OpenGroupRoomByRoomId(roomInfos);
   if (!roomDetails) {
@@ -124,33 +126,11 @@ export async function downloadAttachmentSogsV3(
     throw new Error(`Failed to download attachment. Length is 0 for ${attachment.url}`);
   }
 
-  if (attachment.size === null) {
-    return {
-      ...omit(attachment, 'digest', 'key'),
-      data: dataUint.buffer,
-    };
-  }
-
-  let data = dataUint;
-  if (attachment.size !== dataUint.length) {
-    // we might have padding, check that all the remaining bytes are padding bytes
-    // otherwise we have an error.
-    const unpaddedData = getUnpaddedAttachment(dataUint.buffer, attachment.size);
-    if (!unpaddedData) {
-      throw new Error(
-        `downloadAttachment: Size ${attachment.size} did not match downloaded attachment size ${data.byteLength}`
-      );
-    }
-    data = new Uint8Array(unpaddedData);
-  } else {
-    // nothing to do, the attachment has already the correct size.
-    // There is just no padding included, which is what we agreed on
-    // window?.log?.info('Received opengroupv2 unpadded attachment size:', attachment.size);
-  }
+  const data = validateDownloadedSogsAttachment(attachment, dataUint, options);
 
   return {
     ...omit(attachment, 'digest', 'key'),
-    data: data.buffer,
+    data,
   };
 }
 

ts/receiver/sogsAttachmentDownload.ts

@@ -0,0 +1,56 @@
+import { getUnpaddedAttachment } from '../session/crypto/BufferPadding';
+
+export type SogsAttachmentDownloadOptions = {
+  allowUnknownSize?: boolean;
+};
+
+export function validateDownloadedSogsAttachment(
+  attachment: {
+    url: string;
+    size?: number | null;
+  },
+  dataUint: Uint8Array,
+  options: SogsAttachmentDownloadOptions = {}
+): ArrayBuffer {
+  const data = toExactArrayBuffer(dataUint);
+
+  if (attachment.size === null || attachment.size === undefined) {
+    if (!options.allowUnknownSize) {
+      throw new Error(`downloadAttachmentSogsV3: Missing attachment size for ${attachment.url}`);
+    }
+
+    return data;
+  }
+
+  if (
+    !Number.isFinite(attachment.size) ||
+    !Number.isInteger(attachment.size) ||
+    attachment.size <= 0
+  ) {
+    throw new Error(
+      `downloadAttachmentSogsV3: Invalid attachment size ${attachment.size} for ${attachment.url}`
+    );
+  }
+
+  if (attachment.size !== data.byteLength) {
+    // Payloads may include attachment padding, which is trimmed to the expected size.
+    const unpaddedData = getUnpaddedAttachment(data, attachment.size);
+    if (!unpaddedData) {
+      throw new Error(
+        `downloadAttachment: Size ${attachment.size} did not match downloaded attachment size ${data.byteLength}`
+      );
+    }
+    return unpaddedData;
+  }
+
+  // The attachment already has the expected size, without padding.
+  return data;
+}
+
+function toExactArrayBuffer(data: Uint8Array): ArrayBuffer {
+  if (data.byteOffset === 0 && data.byteLength === data.buffer.byteLength) {
+    return data.buffer;
+  }
+
+  return data.buffer.slice(data.byteOffset, data.byteOffset + data.byteLength);
+}

ts/state/ducks/sogsRoomInfo.tsx

@@ -100,7 +100,9 @@ const roomAvatarChange = createAsyncThunk(
     const { fileUrl } = uploadedFileDetails;
 
     // this is kind of a hack just made to avoid having a specific function downloading from sogs by URL rather than fileID
-    const downloaded = await downloadAttachmentSogsV3({ size: null, url: fileUrl }, roomInfos);
+    const downloaded = await downloadAttachmentSogsV3({ size: null, url: fileUrl }, roomInfos, {
+      allowUnknownSize: true,
+    });
 
     if (!downloaded || !(downloaded.data instanceof ArrayBuffer)) {
       const typeFound = typeof downloaded;

ts/test/receiver/attachments_test.ts

@@ -0,0 +1,54 @@
+import { expect } from 'chai';
+
+import { validateDownloadedSogsAttachment } from '../../receiver/sogsAttachmentDownload';
+
+describe('receiver/attachments', () => {
+  const attachmentUrl = 'https://example.org/file/12345';
+
+  it('rejects a SOGS message attachment without an expected size', () => {
+    expect(() =>
+      validateDownloadedSogsAttachment({ url: attachmentUrl }, new Uint8Array([1, 2, 3]))
+    ).to.throw('Missing attachment size');
+  });
+
+  it('allows unknown size when explicitly requested', () => {
+    const data = new Uint8Array([1, 2, 3]);
+
+    const downloaded = validateDownloadedSogsAttachment({ size: null, url: attachmentUrl }, data, {
+      allowUnknownSize: true,
+    });
+
+    expect(new Uint8Array(downloaded)).to.deep.eq(data);
+  });
+
+  it('rejects a truncated SOGS message attachment', () => {
+    expect(() =>
+      validateDownloadedSogsAttachment({ size: 4, url: attachmentUrl }, new Uint8Array([1, 2, 3]))
+    ).to.throw('did not match downloaded attachment size');
+  });
+
+  it('rejects an invalid SOGS message attachment size', () => {
+    expect(() =>
+      validateDownloadedSogsAttachment({ size: 1.5, url: attachmentUrl }, new Uint8Array([1, 2, 3]))
+    ).to.throw('Invalid attachment size');
+  });
+
+  it('removes padding from oversized SOGS message attachments', () => {
+    const downloaded = validateDownloadedSogsAttachment(
+      { size: 3, url: attachmentUrl },
+      new Uint8Array([1, 2, 3, 0, 0])
+    );
+
+    expect(new Uint8Array(downloaded)).to.deep.eq(new Uint8Array([1, 2, 3]));
+  });
+
+  it('does not include bytes outside the downloaded Uint8Array view', () => {
+    const source = new Uint8Array([9, 1, 2, 3, 9]);
+    const downloaded = validateDownloadedSogsAttachment(
+      { size: 3, url: attachmentUrl },
+      new Uint8Array(source.buffer, 1, 3)
+    );
+
+    expect(new Uint8Array(downloaded)).to.deep.eq(new Uint8Array([1, 2, 3]));
+  });
+});