Server: Added form tokens to prevent CSRF attacks

packages/server/src/app.ts

@@ -135,7 +135,19 @@ async function main() {
 			await next();
 		} catch (error) {
 			ctx.status = error.httpCode || 500;
-			ctx.body = JSON.stringify({ error: error.message });
+
+			// Since this is a low level error, rendering a view might fail too,
+			// so catch this and default to rendering JSON.
+			try {
+				ctx.body = await ctx.joplin.services.mustache.renderView({
+					name: 'error',
+					title: 'Error',
+					path: 'index/error',
+					content: { error },
+				});
+			} catch (anotherError) {
+				ctx.body = { error: anotherError.message };
+			}
 		}
 	});
 

packages/server/src/routes/index/upgrade.ts

@@ -10,6 +10,7 @@ import { bodyFields } from '../../utils/requestUtils';
 import { NotificationKey } from '../../models/NotificationModel';
 import { AccountType } from '../../models/UserModel';
 import { ErrorBadRequest } from '../../utils/errors';
+import { createCsrfTag } from '../../utils/csrf';
 
 interface FormFields {
 	upgrade_button: string;
@@ -21,7 +22,7 @@ function upgradeUrl() {
 	return `${config().baseUrl}/upgrade`;
 }
 
-router.get('upgrade', async (_path: SubPath, _ctx: AppContext) => {
+router.get('upgrade', async (_path: SubPath, ctx: AppContext) => {
 	interface PlanRow {
 		basicLabel: string;
 		proLabel: string;
@@ -51,6 +52,7 @@ router.get('upgrade', async (_path: SubPath, _ctx: AppContext) => {
 		basicPrice: plans.basic.price,
 		proPrice: plans.pro.price,
 		postUrl: upgradeUrl(),
+		csrfTag: await createCsrfTag(ctx),
 	};
 	view.cssFiles = ['index/upgrade'];
 	return view;

packages/server/src/routes/index/users.ts

@@ -17,6 +17,7 @@ import { getCanShareFolder, totalSizeClass } from '../../models/utils/user';
 import { yesNoDefaultOptions } from '../../utils/views/select';
 import { confirmUrl } from '../../utils/urlUtils';
 import { cancelSubscription, updateSubscriptionType } from '../../utils/stripe';
+import { createCsrfTag } from '../../utils/csrf';
 
 export interface CheckRepeatPasswordInput {
 	password: string;
@@ -146,6 +147,7 @@ router.get('users/:id', async (path: SubPath, ctx: AppContext, user: User = null
 	view.content.error = error;
 	view.content.postUrl = postUrl;
 	view.content.showDisableButton = !isNew && !!owner.is_admin && owner.id !== user.id && user.enabled;
+	view.content.csrfTag = await createCsrfTag(ctx);
 
 	if (subscription) {
 		view.content.subscription = subscription;

packages/server/src/utils/csrf.ts

@@ -0,0 +1,37 @@
+import { ErrorForbidden } from './errors';
+import { escapeHtml } from './htmlUtils';
+import { bodyFields, isApiRequest } from './requestUtils';
+import { AppContext } from './types';
+
+interface BodyWithCsrfToken {
+	_csrf: string;
+}
+
+export async function csrfCheck(ctx: AppContext, isPublicRoute: boolean) {
+	if (isApiRequest(ctx)) return;
+	if (isPublicRoute) return;
+	if (!['POST', 'PUT'].includes(ctx.method)) return;
+	if (ctx.path === '/logout') return;
+
+	const userId = ctx.joplin.owner ? ctx.joplin.owner.id : '';
+	if (!userId) return;
+
+	const fields = await bodyFields<BodyWithCsrfToken>(ctx.req);
+	if (!fields._csrf) throw new ErrorForbidden('CSRF token is missing');
+
+	if (!(await ctx.joplin.models.token().isValid(userId, fields._csrf))) {
+		throw new ErrorForbidden(`Invalid CSRF token: ${fields._csrf}`);
+	}
+
+	await ctx.joplin.models.token().deleteByValue(userId, fields._csrf);
+}
+
+export async function createCsrfToken(ctx: AppContext) {
+	if (!ctx.joplin.owner) throw new Error('Cannot create CSRF token without a user');
+	return ctx.joplin.models.token().generate(ctx.joplin.owner.id);
+}
+
+export async function createCsrfTag(ctx: AppContext) {
+	const token = await createCsrfToken(ctx);
+	return `<input type="hidden" name="_csrf" value="${escapeHtml(token)}"/>`;
+}

packages/server/src/utils/requestUtils.ts

@@ -22,6 +22,8 @@ export async function formParse(req: any): Promise<FormParseResult> {
 		return output;
 	}
 
+	if (req.__parsed) return req.__parsed;
+
 	// Note that for Formidable to work, the content-type must be set in the
 	// headers
 	return new Promise((resolve: Function, reject: Function) => {
@@ -32,7 +34,13 @@ export async function formParse(req: any): Promise<FormParseResult> {
 				return;
 			}
 
-			resolve({ fields, files });
+			// Formidable seems to be doing some black magic and once a request
+			// has been parsed it cannot be parsed again. Doing so will do
+			// nothing, the code will just end there, or maybe wait
+			// indefinitely. So we cache the result on success and return it if
+			// some code somewhere tries again to parse the form.
+			req.__parsed = { fields, files };
+			resolve(req.__parsed);
 		});
 	});
 }

packages/server/src/utils/routeUtils.ts

@@ -4,6 +4,7 @@ import { ErrorBadRequest, ErrorForbidden, ErrorNotFound } from './errors';
 import Router from './Router';
 import { AppContext, HttpMethod, RouteType } from './types';
 import { URL } from 'url';
+import { csrfCheck } from './csrf';
 
 const { ltrimSlashes, rtrimSlashes } = require('@joplin/lib/path-utils');
 
@@ -188,10 +189,14 @@ export async function execRequest(routes: Routers, ctx: AppContext) {
 	const endPoint = match.route.findEndPoint(ctx.request.method as HttpMethod, match.subPath.schema);
 	if (ctx.URL && !isValidOrigin(ctx.URL.origin, baseUrl(endPoint.type), endPoint.type)) throw new ErrorNotFound(`Invalid origin: ${ctx.URL.origin}`, 'invalidOrigin');
 
+	const isPublicRoute = match.route.isPublic(match.subPath.schema);
+
 	// This is a generic catch-all for all private end points - if we
 	// couldn't get a valid session, we exit now. Individual end points
 	// might have additional permission checks depending on the action.
-	if (!match.route.isPublic(match.subPath.schema) && !ctx.joplin.owner) throw new ErrorForbidden();
+	if (!isPublicRoute && !ctx.joplin.owner) throw new ErrorForbidden();
+
+	await csrfCheck(ctx, isPublicRoute);
 
 	return endPoint.handler(match.subPath, ctx);
 }

packages/server/src/views/index/upgrade.mustache

@@ -1,7 +1,8 @@
 <h1 class="title">Upgrade your account</h1>
-<p class="subtitle">Upgrading to a Pro account to get the following benefits.</p>
+<p class="subtitle">Upgrade to a Pro account to get the following benefits.</p>
 
 <form id="upgrade_form" action="{{{postUrl}}}" method="POST">
+	{{{csrfTag}}}
 	<table class="table is-hoverable user-props-table">
 		<tbody>
 			<tr>

packages/server/src/views/index/user.mustache

@@ -4,6 +4,7 @@
 
 	<div class="block">
 		{{> errorBanner}}
+		{{{csrfTag}}}
 		<input type="hidden" name="id" value="{{user.id}}"/>
 		<input type="hidden" name="is_new" value="{{isNew}}"/>
 		<div class="field">
@@ -94,11 +95,11 @@
 		</div>
 	</div>
 
-	<h1 class="title">Your subscription</h1>
+	{{#subscription}}
+		<h1 class="title">Your subscription</h1>
 
-	<div class="block">
+		<div class="block">
-		{{#global.owner.is_admin}}
+			{{#global.owner.is_admin}}
-			{{#subscription}}
 				<div class="control block">
 					<p class="block">Stripe Subscription ID: <a href="https://dashboard.stripe.com/subscriptions/{{subscription.stripe_subscription_id}}">{{subscription.stripe_subscription_id}}</a></p>
 					{{#showUpdateSubscriptionBasic}}
@@ -111,11 +112,9 @@
 						<input type="submit" name="cancel_subscription_button" class="button is-danger" value="Cancel subscription" />
 					{{/showCancelSubscription}}
 				</div>
-			{{/subscription}}
+			{{/global.owner.is_admin}}
-		{{/global.owner.is_admin}}
 
-		{{^global.owner.is_admin}}
+			{{^global.owner.is_admin}}
-			{{#subscription}}
 				<div class="control block">
 					{{#showUpdateSubscriptionPro}}
 						<a href="{{{global.baseUrl}}}/upgrade" class="button is-warning block">Upgrade to Pro</a>
@@ -125,9 +124,9 @@
 						<input type="submit" id="user_cancel_subscription_button" name="user_cancel_subscription_button" class="button is-danger" value="Cancel subscription" />
 					{{/showCancelSubscription}}
 				</div>
-			{{/subscription}}
+			{{/global.owner.is_admin}}
-		{{/global.owner.is_admin}}
+		</div>
-	</div>
+	{{/subscription}}
 
 </form>