diff --git a/.changeset/bump-patch-1771425547745.md b/.changeset/bump-patch-1771425547745.md new file mode 100644 index 0000000000000..e1eaa7980afb1 --- /dev/null +++ b/.changeset/bump-patch-1771425547745.md @@ -0,0 +1,5 @@ +--- +'@rocket.chat/meteor': patch +--- + +Bump @rocket.chat/meteor version. diff --git a/.changeset/nice-glasses-love.md b/.changeset/nice-glasses-love.md new file mode 100644 index 0000000000000..eacb88108a0f7 --- /dev/null +++ b/.changeset/nice-glasses-love.md @@ -0,0 +1,5 @@ +--- +'@rocket.chat/meteor': patch +--- + +Security Hotfix (https://docs.rocket.chat/docs/security-fixes-and-updates) diff --git a/apps/meteor/app/api/server/helpers/parseJsonQuery.ts b/apps/meteor/app/api/server/helpers/parseJsonQuery.ts index f21e7b28efe07..9dd6c9c0448af 100644 --- a/apps/meteor/app/api/server/helpers/parseJsonQuery.ts +++ b/apps/meteor/app/api/server/helpers/parseJsonQuery.ts @@ -142,7 +142,8 @@ export async function parseJsonQuery(api: GenericRouteExecutionContext): Promise } } - if (queryFields && !isValidQuery(query, queryFields || ['*'], queryOperations ?? pathAllowConf.def)) { + const containsQueryFields = queryFields.length > 0; + if (containsQueryFields && !isValidQuery(query, queryFields, queryOperations ?? pathAllowConf.def)) { throw new Meteor.Error('error-invalid-query', isValidQuery.errors.join('\n')); } diff --git a/apps/meteor/app/api/server/lib/isValidQuery.ts b/apps/meteor/app/api/server/lib/isValidQuery.ts index 97620cdb1b9ef..ef8a0716505cb 100644 --- a/apps/meteor/app/api/server/lib/isValidQuery.ts +++ b/apps/meteor/app/api/server/lib/isValidQuery.ts @@ -1,3 +1,5 @@ +import { isRecord } from '@rocket.chat/tools'; + import { removeDangerousProps } from './cleanQuery'; type Query = { [k: string]: any }; @@ -8,7 +10,8 @@ export const isValidQuery: { } = Object.assign( (query: Query, allowedAttributes: string[], allowedOperations: string[]): boolean => { isValidQuery.errors = []; - if (!(query instanceof Object)) { + // query is an object with null prototype, so it wont be instance of Object + if (!isRecord(query)) { throw new Error('query must be an object'); } @@ -21,7 +24,7 @@ export const isValidQuery: { ); const verifyQuery = (query: Query, allowedAttributes: string[], allowedOperations: string[], parent = ''): boolean => { - return Object.entries(removeDangerousProps(query)).every(([key, value]) => { + return Object.entries(removeDangerousProps({ ...query })).every(([key, value]) => { const path = parent ? `${parent}.${key}` : key; if (key.startsWith('$')) { if (!allowedOperations.includes(key)) { @@ -33,7 +36,7 @@ const verifyQuery = (query: Query, allowedAttributes: string[], allowedOperation return value.every((v) => verifyQuery(v, allowedAttributes, allowedOperations)); } - if (value instanceof Object) { + if (isRecord(value)) { return verifyQuery(value, allowedAttributes, allowedOperations, path); } @@ -56,7 +59,7 @@ const verifyQuery = (query: Query, allowedAttributes: string[], allowedOperation return false; } - if (value instanceof Object) { + if (isRecord(value)) { return verifyQuery(value, allowedAttributes, allowedOperations, path); } return true; diff --git a/apps/meteor/app/api/server/v1/users.ts b/apps/meteor/app/api/server/v1/users.ts index f57dc2d7bfb8c..229a479d2b9a4 100644 --- a/apps/meteor/app/api/server/v1/users.ts +++ b/apps/meteor/app/api/server/v1/users.ts @@ -511,8 +511,6 @@ API.v1.addRoute( const inclusiveFieldsKeys = Object.keys(inclusiveFields); - const hasUserQuery = query && Object.keys(query).length > 0; - const nonEmptyQuery = getNonEmptyQuery(query, await hasPermissionAsync(this.userId, 'view-full-other-user-info')); // if user provided a query, validate it with their allowed operators @@ -528,7 +526,9 @@ API.v1.addRoute( inclusiveFieldsKeys.includes('type') && 'type.*', inclusiveFieldsKeys.includes('customFields') && 'customFields.*', ].filter(Boolean) as string[], - hasUserQuery ? this.queryOperations : [...this.queryOperations, '$regex', '$options'], + // At this point, we have already validated the user query not containing malicious fields + // On here we are using our own query so we can allow some extra fields + [...this.queryOperations, '$regex', '$options'], ) ) { throw new Meteor.Error('error-invalid-query', isValidQuery.errors.join('\n')); diff --git a/apps/meteor/app/file/server/file.server.ts b/apps/meteor/app/file/server/file.server.ts index 0af01c1bfdab7..629684391bcbd 100644 --- a/apps/meteor/app/file/server/file.server.ts +++ b/apps/meteor/app/file/server/file.server.ts @@ -10,6 +10,8 @@ import { NpmModuleMongodb } from 'meteor/npm-mongo'; import mime from 'mime-type/with-db'; import mkdirp from 'mkdirp'; +import { sanitizeFileName } from './functions/sanitizeFileName'; + const { db } = MongoInternals.defaultRemoteCollectionDriver().mongo; type IFile = { @@ -141,7 +143,7 @@ class FileSystem implements IRocketChatFileStore { } createWriteStream(fileName: string) { - const ws = fs.createWriteStream(path.join(this.absolutePath, fileName)); + const ws = fs.createWriteStream(path.join(this.absolutePath, sanitizeFileName(fileName))); ws.on('close', () => { return ws.emit('end'); }); @@ -149,15 +151,15 @@ class FileSystem implements IRocketChatFileStore { } createReadStream(fileName: string) { - return fs.createReadStream(path.join(this.absolutePath, fileName)); + return fs.createReadStream(path.join(this.absolutePath, sanitizeFileName(fileName))); } stat(fileName: string) { - return fsp.stat(path.join(this.absolutePath, fileName)); + return fsp.stat(path.join(this.absolutePath, sanitizeFileName(fileName))); } async remove(fileName: string) { - return fsp.unlink(path.join(this.absolutePath, fileName)); + return fsp.unlink(path.join(this.absolutePath, sanitizeFileName(fileName))); } async getFileWithReadStream(fileName: string) { diff --git a/apps/meteor/app/file/server/functions/sanitizeFileName.spec.ts b/apps/meteor/app/file/server/functions/sanitizeFileName.spec.ts new file mode 100644 index 0000000000000..bbf09eb718747 --- /dev/null +++ b/apps/meteor/app/file/server/functions/sanitizeFileName.spec.ts @@ -0,0 +1,55 @@ +import { expect } from 'chai'; +import { describe, it } from 'mocha'; + +import { sanitizeFileName } from './sanitizeFileName'; + +describe('sanitizeFileName', () => { + describe('valid filenames', () => { + it('should allow simple filenames', () => { + const result = sanitizeFileName('sound.mp3'); + expect(result).to.equal('sound.mp3'); + }); + + it('should allow filenames with dashes, underscores and dots', () => { + const result = sanitizeFileName('alert_test-01.wav'); + expect(result).to.equal('alert_test-01.wav'); + }); + + it('should allow filenames without extension', () => { + const result = sanitizeFileName('beep'); + expect(result).to.equal('beep'); + }); + }); + + describe('invalid paths', () => { + it('should reject path traversal using ../', () => { + expect(() => sanitizeFileName('../etc/passwd')).to.throw(); + }); + + it('should reject path traversal using ./', () => { + expect(() => sanitizeFileName('./folder/sounds')).to.throw(); + }); + + it('should reject nested paths', () => { + expect(() => sanitizeFileName('sounds/alert.mp3')).to.throw(); + }); + + it('should reject absolute paths', () => { + expect(() => sanitizeFileName('/etc/passwd')).to.throw(); + }); + }); + + describe('invalid characters', () => { + it('should reject filenames with spaces', () => { + expect(() => sanitizeFileName('my sound.mp3')).to.throw(); + }); + + it('should reject filenames with special characters', () => { + expect(() => sanitizeFileName('sound$.mp3')).to.throw(); + }); + + it('should reject filenames with backslashes', () => { + expect(() => sanitizeFileName('..\\passwd')).to.throw(); + }); + }); +}); diff --git a/apps/meteor/app/file/server/functions/sanitizeFileName.ts b/apps/meteor/app/file/server/functions/sanitizeFileName.ts new file mode 100644 index 0000000000000..5b9ca233cbe07 --- /dev/null +++ b/apps/meteor/app/file/server/functions/sanitizeFileName.ts @@ -0,0 +1,19 @@ +import path from 'path'; + +export function sanitizeFileName(fileName: string) { + const base = path.basename(fileName); + + if (base !== fileName) { + throw new Error('error-invalid-file-name'); + } + + if (base === '.' || base.startsWith('..')) { + throw new Error('error-invalid-file-name'); + } + + if (!/^[a-zA-Z0-9._-]+$/.test(base)) { + throw new Error('error-invalid-characters-in-file-name'); + } + + return base; +} diff --git a/docker-compose-ci.yml b/docker-compose-ci.yml index d23feb3fcc435..5f64d30dd5618 100644 --- a/docker-compose-ci.yml +++ b/docker-compose-ci.yml @@ -201,7 +201,7 @@ services: image: kong/httpbin traefik: - image: traefik:v3.1 + image: traefik:v3.6.6 command: - --providers.docker=true - '--serverstransport.maxidleconnsperhost=-1' diff --git a/docker-compose-local.yml b/docker-compose-local.yml index 9e9b3308867f3..64eb4103f3e0a 100644 --- a/docker-compose-local.yml +++ b/docker-compose-local.yml @@ -1,11 +1,6 @@ -version: '3.8' - services: rocketchat: platform: linux/amd64 - build: - dockerfile: apps/meteor/.docker/Dockerfile.alpine - context: /tmp/build image: ghcr.io/${LOWERCASE_REPOSITORY}/rocket.chat:${DOCKER_TAG} environment: - TEST_MODE=true @@ -29,10 +24,6 @@ services: authorization-service: platform: linux/amd64 - build: - dockerfile: ee/apps/authorization-service/Dockerfile - args: - SERVICE: authorization-service image: ghcr.io/${LOWERCASE_REPOSITORY}/authorization-service:${DOCKER_TAG} environment: - 'MONGO_URL=${MONGO_URL}' @@ -45,10 +36,6 @@ services: account-service: platform: linux/amd64 - build: - dockerfile: ee/apps/account-service/Dockerfile - args: - SERVICE: account-service image: ghcr.io/${LOWERCASE_REPOSITORY}/account-service:${DOCKER_TAG} environment: - MONGO_URL=${MONGO_URL} @@ -61,10 +48,6 @@ services: presence-service: platform: linux/amd64 - build: - dockerfile: ee/apps/presence-service/Dockerfile - args: - SERVICE: presence-service image: ghcr.io/${LOWERCASE_REPOSITORY}/presence-service:${DOCKER_TAG} environment: - MONGO_URL=${MONGO_URL} @@ -77,10 +60,6 @@ services: ddp-streamer-service: platform: linux/amd64 - build: - dockerfile: ee/apps/ddp-streamer/Dockerfile - args: - SERVICE: ddp-streamer image: ghcr.io/${LOWERCASE_REPOSITORY}/ddp-streamer-service:${DOCKER_TAG} environment: - MONGO_URL=${MONGO_URL} @@ -99,10 +78,6 @@ services: queue-worker-service: platform: linux/amd64 - build: - dockerfile: ee/apps/queue-worker/Dockerfile - args: - SERVICE: queue-worker image: ghcr.io/${LOWERCASE_REPOSITORY}/queue-worker-service:${DOCKER_TAG} environment: - MONGO_URL=${MONGO_URL} @@ -115,10 +90,6 @@ services: omnichannel-transcript-service: platform: linux/amd64 - build: - dockerfile: ee/apps/omnichannel-transcript/Dockerfile - args: - SERVICE: omnichannel-transcript image: ghcr.io/${LOWERCASE_REPOSITORY}/omnichannel-transcript-service:${DOCKER_TAG} environment: - MONGO_URL=${MONGO_URL} @@ -142,7 +113,7 @@ services: bash -c "mongod --replSet $$MONGODB_REPLICA_SET_NAME --bind_ip_all & sleep 2; - until mongosh --eval \"db.adminCommand('ping')\"; do + until mongosh --eval \"db.adminCommand('ping')\"; do echo '=====> Waiting for Mongo...'; sleep 1; done; @@ -155,9 +126,10 @@ services: image: nats:2.6-alpine traefik: - image: traefik:v2.8 + image: traefik:v3.6.6 command: - --providers.docker=true + - '--serverstransport.maxidleconnsperhost=-1' ports: - 3000:80 volumes: diff --git a/ee/packages/federation-matrix/docker-compose.test.yml b/ee/packages/federation-matrix/docker-compose.test.yml index 8b34c6509cec2..32e271e3cfa4b 100644 --- a/ee/packages/federation-matrix/docker-compose.test.yml +++ b/ee/packages/federation-matrix/docker-compose.test.yml @@ -5,21 +5,21 @@ networks: services: traefik: - image: traefik:v2.9 + image: traefik:v3.6.6 profiles: - test - element command: - - "--api.insecure=true" + - '--api.insecure=true' # - "--log.level=DEBUG" - - "--providers.docker=true" - - "--providers.docker.exposedbydefault=false" - - "--entrypoints.web.address=:80" - - "--entrypoints.websecure.address=:443" + - '--providers.docker=true' + - '--providers.docker.exposedbydefault=false' + - '--entrypoints.web.address=:80' + - '--entrypoints.websecure.address=:443' ports: - - "80:80" - - "443:443" - - "8080:8080" + - '80:80' + - '443:443' + - '8080:8080' volumes: - /var/run/docker.sock:/var/run/docker.sock:ro - ./docker-compose/traefik/traefik.yml:/etc/traefik/traefik.yml:ro @@ -39,7 +39,7 @@ services: element-net: aliases: [hs1, rc1, rc.host, element] -# HomeServer 1 (synapse) + # HomeServer 1 (synapse) hs1: image: matrixdotorg/synapse:latest profiles: @@ -71,13 +71,13 @@ services: networks: - hs1-net labels: - - "traefik.enable=true" - - "traefik.http.routers.hs1.rule=Host(`hs1`)" - - "traefik.http.routers.hs1.entrypoints=websecure" - - "traefik.http.routers.hs1.tls=true" - - "traefik.http.services.hs1.loadbalancer.server.port=8008" + - 'traefik.enable=true' + - 'traefik.http.routers.hs1.rule=Host(`hs1`)' + - 'traefik.http.routers.hs1.entrypoints=websecure' + - 'traefik.http.routers.hs1.tls=true' + - 'traefik.http.services.hs1.loadbalancer.server.port=8008' -# Rocket.Chat rc1 + # Rocket.Chat rc1 rc1: build: context: ${ROCKETCHAT_BUILD_CONTEXT:-./test/dist} @@ -109,15 +109,15 @@ services: depends_on: - mongo labels: - - "traefik.enable=true" - - "traefik.http.routers.rc1.rule=Host(`rc1`)" - - "traefik.http.routers.rc1.entrypoints=websecure" - - "traefik.http.routers.rc1.tls=true" - - "traefik.http.services.rc1.loadbalancer.server.port=3000" + - 'traefik.enable=true' + - 'traefik.http.routers.rc1.rule=Host(`rc1`)' + - 'traefik.http.routers.rc1.entrypoints=websecure' + - 'traefik.http.routers.rc1.tls=true' + - 'traefik.http.services.rc1.loadbalancer.server.port=3000' # HTTPS Redirect - - "traefik.http.middlewares.rc1.redirectscheme.scheme=https" - - "traefik.http.routers.rc1-http.rule=Host(`rc1`)" - - "traefik.http.routers.rc1-http.middlewares=rc1" + - 'traefik.http.middlewares.rc1.redirectscheme.scheme=https' + - 'traefik.http.routers.rc1-http.rule=Host(`rc1`)' + - 'traefik.http.routers.rc1-http.middlewares=rc1' mongo: image: mongo:8.0 @@ -126,7 +126,7 @@ services: - element restart: on-failure ports: - - "27017:27017" + - '27017:27017' entrypoint: | bash -c "mongod --replSet rs0 --bind_ip_all & @@ -153,12 +153,12 @@ services: networks: - element-net labels: - - "traefik.enable=true" - - "traefik.http.routers.element.rule=Host(`element`)" - - "traefik.http.routers.element.entrypoints=websecure" - - "traefik.http.routers.element.tls=true" - - "traefik.http.services.element.loadbalancer.server.port=80" + - 'traefik.enable=true' + - 'traefik.http.routers.element.rule=Host(`element`)' + - 'traefik.http.routers.element.entrypoints=websecure' + - 'traefik.http.routers.element.tls=true' + - 'traefik.http.services.element.loadbalancer.server.port=80' # HTTPS Redirect - - "traefik.http.middlewares.element.redirectscheme.scheme=https" - - "traefik.http.routers.element-http.rule=Host(`element`)" - - "traefik.http.routers.element-http.middlewares=element" + - 'traefik.http.middlewares.element.redirectscheme.scheme=https' + - 'traefik.http.routers.element-http.rule=Host(`element`)' + - 'traefik.http.routers.element-http.middlewares=element' diff --git a/packages/tools/src/isRecord.spec.ts b/packages/tools/src/isRecord.spec.ts index 287dee334e8b8..2047a1843bf1e 100644 --- a/packages/tools/src/isRecord.spec.ts +++ b/packages/tools/src/isRecord.spec.ts @@ -24,7 +24,16 @@ describe('isRecord', () => { test.each([ ['case-1', {}, true], ['case-2', { prop: 'value' }, true], - ['case-2', Object.fromEntries([]), true], + ['case-3', Object.fromEntries([]), true], + ['case-4', Object.create(null), true], + ['case-5', { 0: 'zero', 1: 'one' }, true], + ['case-6', { [Symbol('key')]: 'value' }, true], + // we testing, we ignore eslint + // eslint-disable-next-line no-new-wrappers + ['case-7', new String('string'), false], + // eslint-disable-next-line no-new-wrappers + ['case-8', new Number(1), false], + ['case-9', [], false], ])('should return true for records %# (%s)', (_caseId, input, expected) => { expect(isRecord(input)).toEqual(expected); }); diff --git a/packages/tools/src/isRecord.ts b/packages/tools/src/isRecord.ts index 32ade7ac5f9d2..a55bc78298d9c 100644 --- a/packages/tools/src/isRecord.ts +++ b/packages/tools/src/isRecord.ts @@ -3,5 +3,15 @@ export function isRecord(record: unknown): record is Record