fix: Allow JPEG and GIF images as profile photos #2332

.gitignore

@@ -5,3 +5,4 @@ c4/workspace.json
 .ignore
 .DS_Store
 .vscode
+packages/.DS_Store

CHANGELOG.md

@@ -6,6 +6,10 @@
 
 * Use ack for CREATE_NETWORK and simplify
 
+# Fixes
+
+* Allow JPEG and GIF files as profile photos ([#2332](https://github.com/TryQuiet/quiet/issues/2332))
+
 [2.1.2]
 
 # Refactorings:

package.json

@@ -4,7 +4,10 @@
   "scripts": {
     "lerna": "lerna",
     "publish": "lerna version $npm_config_release --no-private",
-    "postpublish": "node copy-changelog.js && git add . && git commit -m 'Update packages CHANGELOG.md' && git push"
+    "postpublish": "node copy-changelog.js && git add . && git commit -m 'Update packages CHANGELOG.md' && git push",
+    "start:desktop": "lerna run --scope @quiet/desktop start",
+    "lint:all": "lerna run lint",
+    "distAndRunE2ETests:mac:local": "lerna run --scope @quiet/desktop distMac:local && lerna run --scope e2e-tests test:localBinary --"
   },
   "engines": {
     "node": "18.12.1",

packages/backend/.gitignore

@@ -3,4 +3,5 @@
 /coverage
 /testingFixtures/certificates/files
 /testingFixtures/certificates/files2
-/src/storage/testUtils/large-file*
+/src/nest/storage/testUtils/large-file*
+/src/ipfs-file-manager/testUtils/large-file*

packages/backend/CHANGELOG.md

@@ -8,6 +8,10 @@
 
 * Feature: add functionality to export chat to text document in desktop version
 
+* Fix: Updates UserProfileStore to allow JPEG and GIF files ([#2332](https://github.com/TryQuiet/quiet/issues/2332))
+
+* Refactor: Consolidate profile photo validation and match magic byte check to type check
+
 [2.0.3-alpha.6]
 
 * Fix: filter out invalid peer addresses in peer list. Update peer list in localdb.

packages/backend/package.json

@@ -18,7 +18,7 @@
     "lint": "eslint --ext .jsx,.js,.ts,.tsx ./src/ --fix",
     "lint-ci": "eslint --ext .jsx,.js,.ts,.tsx ./src/",
     "test-nest": "cross-env NODE_OPTIONS=--experimental-vm-modules DEBUG=ipfs:*,backend:* node_modules/jest/bin/jest.js --detectOpenHandles --forceExit   ./src/nest/**/*.spec.ts",
-    "test": "cross-env NODE_OPTIONS=--experimental-vm-modules DEBUG=ipfs:*,backend:* node_modules/jest/bin/jest.js ./src/**/* --runInBand --verbose --testPathIgnorePatterns=\".src/(!?nodeTest*)|(.node_modules*)\"",
+    "test": "cross-env NODE_OPTIONS=--experimental-vm-modules DEBUG=ipfs:*,backend:* jest --runInBand --verbose --testPathIgnorePatterns=\".src/(!?nodeTest*)|(.node_modules*)\" --",
     "test-ci": "cross-env NODE_OPTIONS=--experimental-vm-modules jest ./src/**/* --runInBand --colors --ci --silent --verbose --testPathIgnorePatterns=\".src/nest/(!?nodeTest*)|(.node_modules*)|src/nest/.*\\.tor.spec\\.(t|j)s|src/nest/ipfs-file-manager/big-files.long.spec.ts$\"",
     "test-ci-tor": "cross-env NODE_OPTIONS=--experimental-vm-modules jest --runInBand --colors --ci --silent --verbose --detectOpenHandles --forceExit ./src/nest/**/*.tor.spec.ts",
     "test-ci-long-running": "cross-env DEBUG=backend:* NODE_OPTIONS=--experimental-vm-modules jest --colors --ci --verbose ./src/nest/**/*.long.spec.ts",

packages/backend/src/nest/storage/userProfile/userProfile.store.spec.ts

@@ -9,59 +9,7 @@ import { getCrypto, PublicKeyInfo } from 'pkijs'
 import { ChannelMessage, NoCryptoEngineError, PublicChannel, UserProfile } from '@quiet/types'
 import { configCrypto, generateKeyPair, sign } from '@quiet/identity'
 
-import { isPng, base64DataURLToByteArray, UserProfileStore, UserProfileKeyValueIndex } from './userProfile.store'
-
-describe('UserProfileStore/isPng', () => {
-  test('returns true for a valid PNG', () => {
-    // Bytes in decimal copied out of a PNG file
-    // e.g. od -t u1 ~/Pictures/test.png | less
-    const png = new Uint8Array([137, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82])
-    expect(isPng(png)).toBeTruthy()
-  })
-
-  test('returns false for a invalid PNG', () => {
-    // Changed the first byte from 137 to 136
-    const png = new Uint8Array([136, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82])
-    expect(isPng(png)).toBeFalsy()
-  })
-
-  test('returns false for a incomplete PNG', () => {
-    // Removed last byte from the PNG header
-    const png = new Uint8Array([137, 80, 78, 71, 13, 10, 26])
-    expect(isPng(png)).toBeFalsy()
-  })
-})
-
-describe('UserProfileStore/base64DataURLToByteArray', () => {
-  test("throws error if data URL prefix doesn't exist", () => {
-    const contents = '1234567'
-    expect(() => base64DataURLToByteArray(contents)).toThrow(Error)
-  })
-
-  test('throws error if data URL prefix is malformatted', () => {
-    let contents = ',1234567'
-    expect(() => base64DataURLToByteArray(contents)).toThrow(Error)
-
-    contents = ',1234567'
-    expect(() => base64DataURLToByteArray(contents)).toThrow(Error)
-
-    contents = 'data:,1234567'
-    expect(() => base64DataURLToByteArray(contents)).toThrow(Error)
-
-    contents = ';base64,1234567'
-    expect(() => base64DataURLToByteArray(contents)).toThrow(Error)
-
-    contents = 'dat:;base64,1234567'
-    expect(() => base64DataURLToByteArray(contents)).toThrow(Error)
-  })
-
-  test('returns Uint8Array if data URL prefix is correct', () => {
-    // base64 encoding of binary 'm'
-    // btoa('m') == 'bQ=='
-    const contents = 'data:mime;base64,bQ=='
-    expect(base64DataURLToByteArray(contents)).toEqual(new Uint8Array(['m'.charCodeAt(0)]))
-  })
-})
+import { UserProfileStore, UserProfileKeyValueIndex } from './userProfile.store'
 
 const getUserProfile = async ({
   pngByteArray,

packages/backend/src/nest/storage/userProfile/userProfile.store.ts

@@ -17,47 +17,10 @@ import createLogger from '../../common/logger'
 import { OrbitDb } from '../orbitDb/orbitDb.service'
 import { StorageEvents } from '../storage.types'
 import { KeyValueIndex } from '../orbitDb/keyValueIndex'
+import { validatePhoto } from './userProfile.utils'
 
 const logger = createLogger('UserProfileStore')
 
-/**
- * Check magic byte sequence to determine if buffer is a PNG image.
- */
-export const isPng = (buffer: Uint8Array): boolean => {
-  // https://en.wikipedia.org/wiki/PNG
-  const pngHeader = [0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]
-
-  if (buffer.length < pngHeader.length) {
-    return false
-  }
-
-  for (let i = 0; i < pngHeader.length; i++) {
-    if (buffer[i] !== pngHeader[i]) {
-      return false
-    }
-  }
-  return true
-}
-
-/**
- * Takes a base64 data URI string that starts with 'data:*\/*;base64,'
- * as returned from
- * https://developer.mozilla.org/en-US/docs/Web/API/FileReader/readAsDataURL
- * and converts it to a Uint8Array.
- */
-export const base64DataURLToByteArray = (contents: string): Uint8Array => {
-  const [header, base64Data] = contents.split(',')
-  if (!header.startsWith('data:') || !header.endsWith(';base64')) {
-    throw new Error('Expected base64 data URI')
-  }
-  const chars = atob(base64Data)
-  const bytes = new Array(chars.length)
-  for (let i = 0; i < chars.length; i++) {
-    bytes[i] = chars.charCodeAt(i)
-  }
-  return new Uint8Array(bytes)
-}
-
 @Injectable()
 export class UserProfileStore extends EventEmitter {
   public store: KeyValueStore<UserProfile>
@@ -165,33 +128,7 @@ export class UserProfileStore extends EventEmitter {
         return false
       }
 
-      if (typeof profile.photo !== 'string') {
-        logger.error('Expected PNG for user profile photo', userProfile.pubKey)
-        return false
-      }
-
-      if (!profile.photo.startsWith('data:image/png;base64,')) {
-        logger.error('Expected PNG for user profile photo', userProfile.pubKey)
-        return false
-      }
-
-      // We only accept PNG for now. I think some care needs to be used
-      // with the Image element since it can make web requests and
-      // accepts a variety of formats that we may want to limit. Some
-      // interesting thoughts:
-      // https://security.stackexchange.com/a/135636
-      const photoBytes = base64DataURLToByteArray(profile.photo)
-      if (!isPng(photoBytes)) {
-        logger.error('Expected PNG for user profile photo', userProfile.pubKey)
-        return false
-      }
-
-      // 200 KB = 204800 B limit
-      //
-      // TODO: Perhaps the compression matters and we should check
-      // actual dimensions in pixels?
-      if (photoBytes.length > 204800) {
-        logger.error('User profile photo must be less than or equal to 200KB')
+      if (!validatePhoto(profile.photo, userProfile.pubKey)) {
         return false
       }
     } catch (err) {

packages/backend/src/nest/storage/userProfile/userProfile.utils.spec.ts

@@ -0,0 +1,130 @@
+import { describe, expect, test } from '@jest/globals'
+
+import { isPng, base64DataURLToByteArray, isGif, isJpeg, validatePhoto } from './userProfile.utils'
+import { LARGE_IMG_URI, VALID_GIF_URI, VALID_JPEG_URI, VALID_PNG_URI } from './userProfile.utils.spec.const'
+
+describe('isPng', () => {
+  test('returns true for a valid PNG', () => {
+    // Bytes in decimal copied out of a PNG file
+    // e.g. od -t u1 ~/Pictures/test.png | less
+    const png = new Uint8Array([137, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82])
+    expect(isPng(png)).toBeTruthy()
+  })
+
+  test('returns false for a invalid PNG', () => {
+    // Changed the first byte from 137 to 136
+    const png = new Uint8Array([136, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82])
+    expect(isPng(png)).toBeFalsy()
+  })
+
+  test('returns false for a incomplete PNG', () => {
+    // Removed last byte from the PNG header
+    const png = new Uint8Array([137, 80, 78, 71, 13, 10, 26])
+    expect(isPng(png)).toBeFalsy()
+  })
+})
+
+describe('isJpeg', () => {
+  test('returns true for a valid JPEG', () => {
+    const jpeg = new Uint8Array([255, 216, 255, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82])
+    expect(isJpeg(jpeg)).toBeTruthy()
+  })
+
+  test('returns false for a invalid JPEG', () => {
+    const jpeg = new Uint8Array([136, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82])
+    expect(isJpeg(jpeg)).toBeFalsy()
+  })
+
+  test('returns false for a incomplete JPEG', () => {
+    // Removed last byte from the PNG header
+    const jpeg = new Uint8Array([255, 216])
+    expect(isJpeg(jpeg)).toBeFalsy()
+  })
+})
+
+describe('isGif', () => {
+  test('returns true for a valid GIF89', () => {
+    const gif = new Uint8Array([71, 73, 70, 56, 57, 97, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82])
+    expect(isGif(gif)).toBeTruthy()
+  })
+
+  test('returns true for a valid GIF87', () => {
+    const gif = new Uint8Array([71, 73, 70, 56, 55, 97, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82])
+    expect(isGif(gif)).toBeTruthy()
+  })
+
+  test('returns false for a invalid GIF', () => {
+    const gif = new Uint8Array([136, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82])
+    expect(isGif(gif)).toBeFalsy()
+  })
+
+  test('returns false for a incomplete GIF', () => {
+    // Removed last byte from the PNG header
+    const gif = new Uint8Array([71, 73, 70, 56, 57])
+    expect(isGif(gif)).toBeFalsy()
+  })
+})
+
+describe('base64DataURLToByteArray', () => {
+  test("throws error if data URL prefix doesn't exist", () => {
+    const contents = '1234567'
+    expect(() => base64DataURLToByteArray(contents)).toThrow(Error)
+  })
+
+  test('throws error if data URL prefix is malformatted', () => {
+    let contents = ',1234567'
+    expect(() => base64DataURLToByteArray(contents)).toThrow(Error)
+
+    contents = ',1234567'
+    expect(() => base64DataURLToByteArray(contents)).toThrow(Error)
+
+    contents = 'data:,1234567'
+    expect(() => base64DataURLToByteArray(contents)).toThrow(Error)
+
+    contents = ';base64,1234567'
+    expect(() => base64DataURLToByteArray(contents)).toThrow(Error)
+
+    contents = 'dat:;base64,1234567'
+    expect(() => base64DataURLToByteArray(contents)).toThrow(Error)
+  })
+
+  test('returns Uint8Array if data URL prefix is correct', () => {
+    // base64 encoding of binary 'm'
+    // btoa('m') == 'bQ=='
+    const contents = 'data:mime;base64,bQ=='
+    expect(base64DataURLToByteArray(contents)).toEqual(new Uint8Array(['m'.charCodeAt(0)]))
+  })
+})
+
+describe('validatePhoto', () => {
+  test("returns false when the photo isn't a string", () => {
+    const input = 1234 as any
+    expect(validatePhoto(input, 'abc123')).toEqual(false)
+  })
+
+  test("returns false when the photo doesn't have a valid image header", () => {
+    const input = 'Zm9vYmFy'
+    expect(validatePhoto(input, 'abc123')).toEqual(false)
+  })
+
+  test('returns false when the photo is missing the magic byte header', () => {
+    const input = 'data:image/png;base64,Zm9vYmFy'
+    expect(validatePhoto(input, 'abc123')).toEqual(false)
+  })
+
+  test('returns true when the photo is a valid PNG string', () => {
+    expect(validatePhoto(VALID_PNG_URI, 'abc123')).toEqual(true)
+  })
+
+  test('returns true when the photo is a valid JPEG string', () => {
+    expect(validatePhoto(VALID_JPEG_URI, 'abc123')).toEqual(true)
+  })
+
+  test('returns true when the photo is a valid GIF string', () => {
+    expect(validatePhoto(VALID_GIF_URI, 'abc123')).toEqual(true)
+  })
+
+  test('returns false when the photo is larger than 200KB', () => {
+    expect(validatePhoto(LARGE_IMG_URI, 'abc123')).toEqual(false)
+  })
+})