Server: Prevent large data blobs from crashing the application

Ref: brianc/node-postgres#2653

Author: laurent22

packages/server/src/config.ts

@@ -133,6 +133,7 @@ export async function initConfig(envType: Env, env: EnvVariables, overrides: any
 		cookieSecure: env.COOKIES_SECURE,
 		storageDriver: parseStorageDriverConnectionString(env.STORAGE_DRIVER),
 		storageDriverFallback: parseStorageDriverConnectionString(env.STORAGE_DRIVER_FALLBACK),
+		itemSizeHardLimit: 250000000, // Beyond this the Postgres driver will crash the app
 		...overrides,
 	};
 }

packages/server/src/models/BaseModel.ts

@@ -90,6 +90,10 @@ export default abstract class BaseModel<T> {
 		return this.config_.appName;
 	}
 
+	protected get itemSizeHardLimit(): number {
+		return this.config_.itemSizeHardLimit;
+	}
+
 	public get db(): DbConnection {
 		if (this.transactionHandler_.activeTransaction) return this.transactionHandler_.activeTransaction;
 		return this.db_;
@@ -113,7 +117,7 @@ export default abstract class BaseModel<T> {
 		throw new Error('Must be overriden');
 	}
 
-	protected selectFields(options: LoadOptions, defaultFields: string[] = null, mainTable: string = ''): string[] {
+	protected selectFields(options: LoadOptions, defaultFields: string[] = null, mainTable: string = '', requiredFields: string[] = []): string[] {
 		let output: string[] = [];
 		if (options && options.fields) {
 			output = options.fields;
@@ -123,6 +127,12 @@ export default abstract class BaseModel<T> {
 			output = this.defaultFields;
 		}
 
+		if (!output.includes('*')) {
+			for (const f of requiredFields) {
+				if (!output.includes(f)) output.push(f);
+			}
+		}
+
 		if (mainTable) {
 			output = output.map(f => {
 				if (f.includes(`${mainTable}.`)) return f;

packages/server/src/models/ItemModel.test.ts

@@ -1,11 +1,12 @@
-import { createUserAndSession, beforeAllDb, afterAllTests, beforeEachDb, models, createItem, createItemTree, createResource, createNote, createFolder, createItemTree3, db, tempDir } from '../utils/testing/testUtils';
+import { createUserAndSession, beforeAllDb, afterAllTests, beforeEachDb, models, createItem, createItemTree, createResource, createNote, createFolder, createItemTree3, db, tempDir, expectNotThrow, expectHttpError } from '../utils/testing/testUtils';
 import { shareFolderWithUser } from '../utils/testing/shareApiUtils';
 import { resourceBlobPath } from '../utils/joplinUtils';
 import newModelFactory from './factory';
 import { StorageDriverType } from '../utils/types';
 import config from '../config';
 import { msleep } from '../utils/time';
 import loadStorageDriver from './items/storage/loadStorageDriver';
+import { ErrorPayloadTooLarge } from '../utils/errors';
 
 describe('ItemModel', function() {
 
@@ -270,6 +271,36 @@ describe('ItemModel', function() {
 		expect((await models().user().load(user3.id)).total_item_size).toBe(expected3);
 	});
 
+	test('should respect the hard item size limit', async function() {
+		const { user: user1 } = await createUserAndSession(1);
+
+		let models = newModelFactory(db(), config());
+
+		let result = await models.item().saveFromRawContent(user1, {
+			body: Buffer.from('1234'),
+			name: 'test1.txt',
+		});
+
+		const item = result['test1.txt'].item;
+
+		config().itemSizeHardLimit = 3;
+		models = newModelFactory(db(), config());
+
+		result = await models.item().saveFromRawContent(user1, {
+			body: Buffer.from('1234'),
+			name: 'test2.txt',
+		});
+
+		expect(result['test2.txt'].error.httpCode).toBe(ErrorPayloadTooLarge.httpCode);
+
+		await expectHttpError(async () => models.item().loadWithContent(item.id), ErrorPayloadTooLarge.httpCode);
+
+		config().itemSizeHardLimit = 1000;
+		models = newModelFactory(db(), config());
+
+		await expectNotThrow(async () => models.item().loadWithContent(item.id));
+	});
+
 	test('should allow importing content to item storage', async function() {
 		const { user: user1 } = await createUserAndSession(1);
 

packages/server/src/models/ItemModel.ts

@@ -3,7 +3,7 @@ import { ItemType, databaseSchema, Uuid, Item, ShareType, Share, ChangeType, Use
 import { defaultPagination, paginateDbQuery, PaginatedResults, Pagination } from './utils/pagination';
 import { isJoplinItemName, isJoplinResourceBlobPath, linkedResourceIds, serializeJoplinItem, unserializeJoplinItem } from '../utils/joplinUtils';
 import { ModelType } from '@joplin/lib/BaseModel';
-import { ApiError, ErrorCode, ErrorForbidden, ErrorUnprocessableEntity } from '../utils/errors';
+import { ApiError, ErrorCode, ErrorForbidden, ErrorPayloadTooLarge, ErrorUnprocessableEntity } from '../utils/errors';
 import { Knex } from 'knex';
 import { ChangePreviousItem } from './ChangeModel';
 import { unique } from '../utils/array';
@@ -14,6 +14,7 @@ import { NewModelFactoryHandler } from './factory';
 import loadStorageDriver from './items/storage/loadStorageDriver';
 import { msleep } from '../utils/time';
 import Logger from '@joplin/lib/Logger';
+import prettyBytes = require('pretty-bytes');
 
 const mimeUtils = require('@joplin/lib/mime-utils.js').mime;
 
@@ -182,7 +183,11 @@ export default class ItemModel extends BaseModel<Item> {
 		}
 	}
 
-	private async storageDriverRead(itemId: Uuid, context: Context) {
+	private async storageDriverRead(itemId: Uuid, itemSize: number, context: Context) {
+		if (itemSize > this.itemSizeHardLimit) {
+			throw new ErrorPayloadTooLarge(`Downloading items larger than ${prettyBytes(this.itemSizeHardLimit)} is currently disabled`);
+		}
+
 		const storageDriver = await this.storageDriver();
 		const storageDriverFallback = await this.storageDriverFallback();
 
@@ -203,13 +208,13 @@ export default class ItemModel extends BaseModel<Item> {
 		const rows: Item[] = await this
 			.db('user_items')
 			.leftJoin('items', 'items.id', 'user_items.item_id')
-			.distinct(this.selectFields(options, null, 'items'))
+			.distinct(this.selectFields(options, null, 'items', ['items.content_size']))
 			.whereIn('user_items.user_id', userIds)
 			.whereIn('jop_id', jopIds);
 
 		if (options.withContent) {
 			for (const row of rows) {
-				row.content = await this.storageDriverRead(row.id, { models: this.models() });
+				row.content = await this.storageDriverRead(row.id, row.content_size, { models: this.models() });
 			}
 		}
 
@@ -229,13 +234,13 @@ export default class ItemModel extends BaseModel<Item> {
 		const rows: Item[] = await this
 			.db('user_items')
 			.leftJoin('items', 'items.id', 'user_items.item_id')
-			.distinct(this.selectFields(options, null, 'items'))
+			.distinct(this.selectFields(options, null, 'items', ['items.content_size']))
 			.whereIn('user_items.user_id', userIds)
 			.whereIn('name', names);
 
 		if (options.withContent) {
 			for (const row of rows) {
-				row.content = await this.storageDriverRead(row.id, { models: this.models() });
+				row.content = await this.storageDriverRead(row.id, row.content_size, { models: this.models() });
 			}
 		}
 
@@ -248,15 +253,17 @@ export default class ItemModel extends BaseModel<Item> {
 	}
 
 	public async loadWithContent(id: Uuid, options: ItemLoadOptions = {}): Promise<Item> {
-		const content = await this.storageDriverRead(id, { models: this.models() });
+		const item: Item = await this
+			.db('user_items')
+			.leftJoin('items', 'items.id', 'user_items.item_id')
+			.select(this.selectFields(options, ['*'], 'items', ['items.content_size']))
+			.where('items.id', '=', id)
+			.first();
+
+		const content = await this.storageDriverRead(id, item.content_size, { models: this.models() });
 
 		return {
-			...await this
-				.db('user_items')
-				.leftJoin('items', 'items.id', 'user_items.item_id')
-				.select(this.selectFields(options, ['*'], 'items'))
-				.where('items.id', '=', id)
-				.first(),
+			...item,
 			content,
 		};
 	}

packages/server/src/models/UserModel.ts

@@ -26,6 +26,7 @@ import paymentFailedAccountDisabledTemplate from '../views/emails/paymentFailedA
 import changeEmailConfirmationTemplate from '../views/emails/changeEmailConfirmationTemplate';
 import changeEmailNotificationTemplate from '../views/emails/changeEmailNotificationTemplate';
 import { NotificationKey } from './NotificationModel';
+import prettyBytes = require('pretty-bytes');
 
 const logger = Logger.create('UserModel');
 
@@ -197,6 +198,17 @@ export default class UserModel extends BaseModel<User> {
 
 		const maxItemSize = getMaxItemSize(user);
 		const maxSize = maxItemSize * (itemIsEncrypted(item) ? 2.2 : 1);
+
+		if (itemSize > 200000000) {
+			logger.info(`Trying to upload large item: ${JSON.stringify({
+				userId: user.id,
+				itemName: item.name,
+				itemSize,
+				maxItemSize,
+				maxSize,
+			}, null, '    ')}`);
+		}
+
 		if (maxSize && itemSize > maxSize) {
 			throw new ErrorPayloadTooLarge(_('Cannot save %s "%s" because it is larger than the allowed limit (%s)',
 				isNote ? _('note') : _('attachment'),
@@ -205,6 +217,8 @@ export default class UserModel extends BaseModel<User> {
 			));
 		}
 
+		if (itemSize > this.itemSizeHardLimit) throw new ErrorPayloadTooLarge(`Uploading items larger than ${prettyBytes(this.itemSizeHardLimit)} is currently disabled`);
+
 		// We allow lock files to go through so that sync can happen, which in
 		// turns allow user to fix oversized account by deleting items.
 		const isWhiteListed = itemSize < 200 && item.name.startsWith('locks/');

packages/server/src/utils/types.ts

@@ -159,6 +159,7 @@ export interface Config {
 	cookieSecure: boolean;
 	storageDriver: StorageDriverConfig;
 	storageDriverFallback: StorageDriverConfig;
+	itemSizeHardLimit: number;
 }
 
 export enum HttpMethod {