steps towards more proper csrf validation

This commit is contained in:
Evert Prants 2022-03-20 17:06:04 +02:00
parent c35cb362ff
commit 21a1ceeb2d
Signed by: evert
GPG Key ID: 1688DA83D222D0B5
8 changed files with 69 additions and 29 deletions

67
package-lock.json generated
View File

@ -16,12 +16,12 @@
"@nestjs/serve-static": "^2.2.2",
"@nestjs/throttler": "^2.0.1",
"bcrypt": "^5.0.1",
"body-parser": "^1.19.2",
"class-transformer": "^0.5.1",
"class-validator": "^0.13.2",
"connect-redis": "^6.1.3",
"cookie-parser": "^1.4.6",
"cropperjs": "^1.5.12",
"csrf": "^3.1.0",
"dotenv": "^16.0.0",
"express-session": "^1.17.2",
"image-size": "^1.0.1",
@ -49,7 +49,6 @@
"@nestjs/testing": "^8.0.0",
"@types/bcrypt": "^5.0.0",
"@types/connect-redis": "^0.0.18",
"@types/csurf": "^1.11.2",
"@types/express": "^4.17.13",
"@types/express-session": "^1.17.4",
"@types/jest": "27.4.1",
@ -3153,15 +3152,6 @@
"integrity": "sha512-t73xJJrvdTjXrn4jLS9VSGRbz0nUY3cl2DMGDU48lKl+HR9dbbjW2A9r3g40VA++mQpy6uuHg33gy7du2BKpog==",
"dev": true
},
"node_modules/@types/csurf": {
"version": "1.11.2",
"resolved": "https://registry.npmjs.org/@types/csurf/-/csurf-1.11.2.tgz",
"integrity": "sha512-9bc98EnwmC1S0aSJiA8rWwXtgXtXHHOQOsGHptImxFgqm6CeH+mIOunHRg6+/eg2tlmDMX3tY7XrWxo2M/nUNQ==",
"dev": true,
"dependencies": {
"@types/express-serve-static-core": "*"
}
},
"node_modules/@types/eslint": {
"version": "8.4.1",
"resolved": "https://registry.npmjs.org/@types/eslint/-/eslint-8.4.1.tgz",
@ -5220,6 +5210,19 @@
"node": "*"
}
},
"node_modules/csrf": {
"version": "3.1.0",
"resolved": "https://registry.npmjs.org/csrf/-/csrf-3.1.0.tgz",
"integrity": "sha512-uTqEnCvWRk042asU6JtapDTcJeeailFy4ydOQS28bj1hcLnYRiqi8SsD2jS412AY1I/4qdOwWZun774iqywf9w==",
"dependencies": {
"rndm": "1.2.0",
"tsscmp": "1.0.6",
"uid-safe": "2.1.5"
},
"engines": {
"node": ">= 0.8"
}
},
"node_modules/css-loader": {
"version": "6.7.1",
"resolved": "https://registry.npmjs.org/css-loader/-/css-loader-6.7.1.tgz",
@ -10313,6 +10316,11 @@
"url": "https://github.com/sponsors/isaacs"
}
},
"node_modules/rndm": {
"version": "1.2.0",
"resolved": "https://registry.npmjs.org/rndm/-/rndm-1.2.0.tgz",
"integrity": "sha1-8z/pz7Urv9UgqhgyO8ZdsRCht2w="
},
"node_modules/run-async": {
"version": "2.4.1",
"resolved": "https://registry.npmjs.org/run-async/-/run-async-2.4.1.tgz",
@ -11501,6 +11509,14 @@
"resolved": "https://registry.npmjs.org/tslib/-/tslib-2.3.1.tgz",
"integrity": "sha512-77EbyPPpMz+FRFRuAFlWMtmgUWGe9UOG2Z25NqCwiIjRhOf5iKGuzSe5P2w1laq+FkRy4p+PCuVkJSGkzTEKVw=="
},
"node_modules/tsscmp": {
"version": "1.0.6",
"resolved": "https://registry.npmjs.org/tsscmp/-/tsscmp-1.0.6.tgz",
"integrity": "sha512-LxhtAkPDTkVCMQjt2h6eBVY28KCjikZqZfMcC15YBeNjkgUpdCfBu5HoiOTDu86v6smE8yOjyEktJ8hlbANHQA==",
"engines": {
"node": ">=0.6.x"
}
},
"node_modules/tsutils": {
"version": "3.21.0",
"resolved": "https://registry.npmjs.org/tsutils/-/tsutils-3.21.0.tgz",
@ -14747,15 +14763,6 @@
"integrity": "sha512-t73xJJrvdTjXrn4jLS9VSGRbz0nUY3cl2DMGDU48lKl+HR9dbbjW2A9r3g40VA++mQpy6uuHg33gy7du2BKpog==",
"dev": true
},
"@types/csurf": {
"version": "1.11.2",
"resolved": "https://registry.npmjs.org/@types/csurf/-/csurf-1.11.2.tgz",
"integrity": "sha512-9bc98EnwmC1S0aSJiA8rWwXtgXtXHHOQOsGHptImxFgqm6CeH+mIOunHRg6+/eg2tlmDMX3tY7XrWxo2M/nUNQ==",
"dev": true,
"requires": {
"@types/express-serve-static-core": "*"
}
},
"@types/eslint": {
"version": "8.4.1",
"resolved": "https://registry.npmjs.org/@types/eslint/-/eslint-8.4.1.tgz",
@ -16397,6 +16404,16 @@
"resolved": "https://registry.npmjs.org/crypt/-/crypt-0.0.2.tgz",
"integrity": "sha1-iNf/fsDfuG9xPch7u0LQRNPmxBs="
},
"csrf": {
"version": "3.1.0",
"resolved": "https://registry.npmjs.org/csrf/-/csrf-3.1.0.tgz",
"integrity": "sha512-uTqEnCvWRk042asU6JtapDTcJeeailFy4ydOQS28bj1hcLnYRiqi8SsD2jS412AY1I/4qdOwWZun774iqywf9w==",
"requires": {
"rndm": "1.2.0",
"tsscmp": "1.0.6",
"uid-safe": "2.1.5"
}
},
"css-loader": {
"version": "6.7.1",
"resolved": "https://registry.npmjs.org/css-loader/-/css-loader-6.7.1.tgz",
@ -20279,6 +20296,11 @@
"glob": "^7.1.3"
}
},
"rndm": {
"version": "1.2.0",
"resolved": "https://registry.npmjs.org/rndm/-/rndm-1.2.0.tgz",
"integrity": "sha1-8z/pz7Urv9UgqhgyO8ZdsRCht2w="
},
"run-async": {
"version": "2.4.1",
"resolved": "https://registry.npmjs.org/run-async/-/run-async-2.4.1.tgz",
@ -21126,6 +21148,11 @@
"resolved": "https://registry.npmjs.org/tslib/-/tslib-2.3.1.tgz",
"integrity": "sha512-77EbyPPpMz+FRFRuAFlWMtmgUWGe9UOG2Z25NqCwiIjRhOf5iKGuzSe5P2w1laq+FkRy4p+PCuVkJSGkzTEKVw=="
},
"tsscmp": {
"version": "1.0.6",
"resolved": "https://registry.npmjs.org/tsscmp/-/tsscmp-1.0.6.tgz",
"integrity": "sha512-LxhtAkPDTkVCMQjt2h6eBVY28KCjikZqZfMcC15YBeNjkgUpdCfBu5HoiOTDu86v6smE8yOjyEktJ8hlbANHQA=="
},
"tsutils": {
"version": "3.21.0",
"resolved": "https://registry.npmjs.org/tsutils/-/tsutils-3.21.0.tgz",

View File

@ -30,12 +30,12 @@
"@nestjs/serve-static": "^2.2.2",
"@nestjs/throttler": "^2.0.1",
"bcrypt": "^5.0.1",
"body-parser": "^1.19.2",
"class-transformer": "^0.5.1",
"class-validator": "^0.13.2",
"connect-redis": "^6.1.3",
"cookie-parser": "^1.4.6",
"cropperjs": "^1.5.12",
"csrf": "^3.1.0",
"dotenv": "^16.0.0",
"express-session": "^1.17.2",
"image-size": "^1.0.1",
@ -63,7 +63,6 @@
"@nestjs/testing": "^8.0.0",
"@types/bcrypt": "^5.0.0",
"@types/connect-redis": "^0.0.18",
"@types/csurf": "^1.11.2",
"@types/express": "^4.17.13",
"@types/express-session": "^1.17.4",
"@types/jest": "27.4.1",

View File

@ -9,8 +9,11 @@ export class CSRFMiddleware implements NestMiddleware {
use(req: Request, res: Response, next: NextFunction) {
// TODO: do not store in session, keep the amount of pointless sessions down
if (!req.session.csrf) {
req.session.csrf = this.tokenService.generateString(64);
req.session.csrf = this.tokenService.csrf.secretSync();
}
req.csrfToken = () => this.tokenService.csrf.create(req.session.csrf);
next();
}
}

View File

@ -1,15 +1,18 @@
import { Injectable, NestMiddleware } from '@nestjs/common';
import { NextFunction, Request, Response } from 'express';
import { TokenService } from 'src/modules/utility/services/token.service';
@Injectable()
export class ValidateCSRFMiddleware implements NestMiddleware {
constructor(private readonly tokenService: TokenService) {}
use(req: Request, res: Response, next: NextFunction) {
// Multipart is handeled elsewhere
if (req.header('content-type')?.startsWith('multipart/form-data')) {
return next();
}
if (req.body._csrf !== req.session.csrf) {
if (!this.tokenService.verifyCSRF(req)) {
return next(new Error('Invalid session'));
}

View File

@ -2,7 +2,6 @@ import {
BadRequestException,
Body,
Controller,
Delete,
Get,
Param,
Post,
@ -10,19 +9,18 @@ import {
Render,
Req,
Res,
Session,
UnauthorizedException,
UploadedFile,
UseInterceptors,
} from '@nestjs/common';
import { FileInterceptor } from '@nestjs/platform-express';
import { Request, Response } from 'express';
import { SessionData } from 'express-session';
import { unlink } from 'fs/promises';
import { OAuth2ClientService } from 'src/modules/objects/oauth2-client/oauth2-client.service';
import { UploadService } from 'src/modules/objects/upload/upload.service';
import { UserService } from 'src/modules/objects/user/user.service';
import { FormUtilityService } from 'src/modules/utility/services/form-utility.service';
import { TokenService } from 'src/modules/utility/services/token.service';
import { SettingsService } from './settings.service';
@Controller('/account')
@ -31,6 +29,7 @@ export class SettingsController {
private readonly _service: SettingsService,
private readonly _form: FormUtilityService,
private readonly _upload: UploadService,
private readonly _token: TokenService,
private readonly _user: UserService,
private readonly _client: OAuth2ClientService,
) {}
@ -87,7 +86,7 @@ export class SettingsController {
@Req() req: Request,
@UploadedFile() file: Express.Multer.File,
) {
if (req.body.csrf !== req.session.csrf) {
if (!this._token.verifyCSRF(req)) {
throw new BadRequestException('Invalid session. Please try again.');
}

View File

@ -33,7 +33,7 @@ export class FormUtilityService {
return {
path: req.originalUrl,
csrf: req.session.csrf,
csrf: req.csrfToken(),
message,
form,
...additional,

View File

@ -2,14 +2,22 @@ import { Injectable } from '@nestjs/common';
import * as crypto from 'crypto';
import { ConfigurationService } from 'src/modules/config/config.service';
import { v4 } from 'uuid';
import * as CSRF from 'csrf';
import { Request } from 'express';
const IV_LENGTH = 16;
const ALGORITHM = 'aes-256-cbc';
@Injectable()
export class TokenService {
public csrf = new CSRF();
constructor(private config: ConfigurationService) {}
public verifyCSRF(req: Request): boolean {
return this.csrf.verify(req.session.csrf, req.body._csrf);
}
public generateString(length: number): string {
return crypto.randomBytes(length).toString('hex').slice(0, length);
}

View File

@ -6,6 +6,7 @@ declare global {
export interface Request {
oauth2: OAuth2;
user: User;
csrfToken: () => string;
flash: (type: string, ...msg: any[]) => Record<string, any>;
}
}