From 186bd744593a9cce27b6b892f0b4898fd6ed9704 Mon Sep 17 00:00:00 2001 From: Inian Parameshwaran Date: Thu, 18 Mar 2021 15:30:06 +0530 Subject: [PATCH] fix: validate object and bucket names --- src/routes/bucket/createBucket.ts | 10 +++++++++- src/routes/object/copyObject.ts | 10 +++++++++- src/routes/object/createObject.ts | 10 +++++++++- src/routes/object/deleteObject.ts | 10 +++++++++- src/routes/object/getObject.ts | 10 +++++++++- src/routes/object/getSignedURL.ts | 2 +- src/routes/object/renameObject.ts | 10 +++++++++- src/routes/object/updateObject.ts | 10 +++++++++- src/test/object.test.ts | 4 ++-- src/utils/index.ts | 6 ++++++ 10 files changed, 72 insertions(+), 10 deletions(-) diff --git a/src/routes/bucket/createBucket.ts b/src/routes/bucket/createBucket.ts index 065866f5..b9e78aca 100644 --- a/src/routes/bucket/createBucket.ts +++ b/src/routes/bucket/createBucket.ts @@ -1,5 +1,5 @@ import { FastifyInstance } from 'fastify' -import { getPostgrestClient, getOwner, transformPostgrestError } from '../../utils' +import { getPostgrestClient, getOwner, transformPostgrestError, isValidKey } from '../../utils' import { AuthenticatedRequest, Bucket } from '../../types/types' import { FromSchema } from 'json-schema-to-ts' import { bucketSchema } from '../../schemas/bucket' @@ -44,6 +44,14 @@ export default async function routes(fastify: FastifyInstance) { id = bucketName } + if (!isValidKey(id) || !isValidKey(bucketName)) { + return response.status(400).send({ + statusCode: '400', + error: 'Invalid key', + message: 'The key contains invalid characters', + }) + } + const { data: results, error, status } = await postgrest .from('buckets') .insert([ diff --git a/src/routes/object/copyObject.ts b/src/routes/object/copyObject.ts index 069b5d32..ecd4f05c 100644 --- a/src/routes/object/copyObject.ts +++ b/src/routes/object/copyObject.ts @@ -1,5 +1,5 @@ import { FastifyInstance } from 'fastify' -import { getPostgrestClient, transformPostgrestError } from '../../utils' +import { getPostgrestClient, isValidKey, transformPostgrestError } from '../../utils' import { copyObject, initClient } from '../../utils/s3' import { getConfig } from '../../utils/config' import { Obj, AuthenticatedRequest } from '../../types/types' @@ -48,6 +48,14 @@ export default async function routes(fastify: FastifyInstance) { const { sourceKey, destinationKey, bucketName } = request.body console.log(sourceKey, bucketName) + if (!isValidKey(destinationKey)) { + return response.status(400).send({ + statusCode: '400', + error: 'Invalid key', + message: 'The destination key contains invalid characters', + }) + } + const postgrest = getPostgrestClient(jwt) const objectResponse = await postgrest .from('objects') diff --git a/src/routes/object/createObject.ts b/src/routes/object/createObject.ts index e1657803..b18c8d10 100644 --- a/src/routes/object/createObject.ts +++ b/src/routes/object/createObject.ts @@ -1,5 +1,5 @@ import { FastifyInstance } from 'fastify' -import { getPostgrestClient, getOwner, transformPostgrestError } from '../../utils' +import { getPostgrestClient, getOwner, transformPostgrestError, isValidKey } from '../../utils' import { uploadObject, initClient, deleteObject } from '../../utils/s3' import { getConfig } from '../../utils/config' import { Obj, AuthenticatedRequest } from '../../types/types' @@ -55,6 +55,14 @@ export default async function routes(fastify: FastifyInstance) { const { bucketName } = request.params const objectName = request.params['*'] + if (!isValidKey(objectName) || !isValidKey(bucketName)) { + return response.status(400).send({ + statusCode: '400', + error: 'Invalid key', + message: 'The key contains invalid characters', + }) + } + const postgrest = getPostgrestClient(jwt) let owner try { diff --git a/src/routes/object/deleteObject.ts b/src/routes/object/deleteObject.ts index d9459b26..88f9d1c1 100644 --- a/src/routes/object/deleteObject.ts +++ b/src/routes/object/deleteObject.ts @@ -1,5 +1,5 @@ import { FastifyInstance } from 'fastify' -import { getPostgrestClient, transformPostgrestError } from '../../utils' +import { getPostgrestClient, isValidKey, transformPostgrestError } from '../../utils' import { deleteObject, initClient } from '../../utils/s3' import { getConfig } from '../../utils/config' import { Obj, AuthenticatedRequest } from '../../types/types' @@ -49,6 +49,14 @@ export default async function routes(fastify: FastifyInstance) { const postgrest = getPostgrestClient(jwt) + if (!isValidKey(objectName) || !isValidKey(bucketName)) { + return response.status(400).send({ + statusCode: '400', + error: 'Invalid key', + message: 'The key contains invalid characters', + }) + } + // todo what if objectName is * or something const objectResponse = await postgrest .from('objects') diff --git a/src/routes/object/getObject.ts b/src/routes/object/getObject.ts index 9ab6d572..dd8b6edf 100644 --- a/src/routes/object/getObject.ts +++ b/src/routes/object/getObject.ts @@ -1,6 +1,6 @@ import { FastifyInstance } from 'fastify' import { FromSchema } from 'json-schema-to-ts' -import { getPostgrestClient, transformPostgrestError } from '../../utils' +import { getPostgrestClient, isValidKey, transformPostgrestError } from '../../utils' import { getObject, initClient } from '../../utils/s3' import { getConfig } from '../../utils/config' import { AuthenticatedRequest, Obj } from '../../types/types' @@ -42,6 +42,14 @@ export default async function routes(fastify: FastifyInstance) { const { bucketName } = request.params const objectName = request.params['*'] + if (!isValidKey(objectName) || !isValidKey(bucketName)) { + return response.status(400).send({ + statusCode: '400', + error: 'Invalid key', + message: 'The key contains invalid characters', + }) + } + const objectResponse = await postgrest .from('objects') .select('*') diff --git a/src/routes/object/getSignedURL.ts b/src/routes/object/getSignedURL.ts index 44687749..fe9292df 100644 --- a/src/routes/object/getSignedURL.ts +++ b/src/routes/object/getSignedURL.ts @@ -56,7 +56,7 @@ export default async function routes(fastify: FastifyInstance) { const objectResponse = await postgrest .from('objects') - .select('*, buckets(*)') + .select('*') .match({ name: objectName, bucket_id: bucketName, diff --git a/src/routes/object/renameObject.ts b/src/routes/object/renameObject.ts index 1ad2093b..0e4cae99 100644 --- a/src/routes/object/renameObject.ts +++ b/src/routes/object/renameObject.ts @@ -1,5 +1,5 @@ import { FastifyInstance } from 'fastify' -import { getPostgrestClient, transformPostgrestError } from '../../utils' +import { getPostgrestClient, isValidKey, transformPostgrestError } from '../../utils' import { initClient, copyObject, deleteObject } from '../../utils/s3' import { getConfig } from '../../utils/config' import { Obj, AuthenticatedRequest } from '../../types/types' @@ -48,6 +48,14 @@ export default async function routes(fastify: FastifyInstance) { const { destinationKey, sourceKey, bucketName } = request.body + if (!isValidKey(destinationKey)) { + return response.status(400).send({ + statusCode: '400', + error: 'Invalid key', + message: 'The destination key contains invalid characters', + }) + } + const postgrest = getPostgrestClient(jwt) const objectResponse = await postgrest diff --git a/src/routes/object/updateObject.ts b/src/routes/object/updateObject.ts index a75f9b1c..b6dba4cf 100644 --- a/src/routes/object/updateObject.ts +++ b/src/routes/object/updateObject.ts @@ -1,5 +1,5 @@ import { FastifyInstance } from 'fastify' -import { getPostgrestClient, getOwner, transformPostgrestError } from '../../utils' +import { getPostgrestClient, getOwner, transformPostgrestError, isValidKey } from '../../utils' import { uploadObject, initClient } from '../../utils/s3' import { getConfig } from '../../utils/config' import { Obj, AuthenticatedRequest } from '../../types/types' @@ -53,6 +53,14 @@ export default async function routes(fastify: FastifyInstance) { const { bucketName } = request.params const objectName = request.params['*'] + if (!isValidKey(objectName) || !isValidKey(bucketName)) { + return response.status(400).send({ + statusCode: '400', + error: 'Invalid key', + message: 'The key contains invalid characters', + }) + } + const postgrest = getPostgrestClient(jwt) const owner = await getOwner(jwt) diff --git a/src/test/object.test.ts b/src/test/object.test.ts index 3e61a9be..a91ddb4b 100644 --- a/src/test/object.test.ts +++ b/src/test/object.test.ts @@ -527,7 +527,7 @@ describe('testing deleting multiple objects', () => { expect(mockDeleteObjects).not.toHaveBeenCalled() }) - test('return 400 when delete from a non existent bucket', async () => { + test('deleting from a non existent bucket', async () => { const response = await app().inject({ method: 'DELETE', url: '/object/notfound', @@ -538,7 +538,7 @@ describe('testing deleting multiple objects', () => { prefixes: ['authenticated/delete-multiple3.png', 'authenticated/delete-multiple4.png'], }, }) - expect(response.statusCode).toBe(400) + expect(response.statusCode).toBe(200) expect(mockDeleteObjects).not.toHaveBeenCalled() }) diff --git a/src/utils/index.ts b/src/utils/index.ts index 513b746a..4d10e748 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -67,3 +67,9 @@ export function transformPostgrestError( message, } } + +export function isValidKey(key: string): boolean { + // only allow s3 safe characters and characters which require special handling for now + // https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-keys.html + return /^(\w|\/|!|-|\.|\*|'|\(|\)| |&|\$|@|=|;|:|\+|,|\?)*$/.test(key) +}