Skip to content

Commit

Permalink
fix(core): Do not allow arbitrary path traversal in the credential-tr…
Browse files Browse the repository at this point in the history
…anslation endpoint (#5522)
  • Loading branch information
netroy committed Feb 21, 2023
1 parent 26a20ed commit f0f8d59
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 59 deletions.
45 changes: 2 additions & 43 deletions packages/cli/src/Server.ts
Expand Up @@ -57,7 +57,6 @@ import history from 'connect-history-api-fallback';
import config from '@/config';
import * as Queue from '@/Queue';
import { InternalHooksManager } from '@/InternalHooksManager';
import { getCredentialTranslationPath } from '@/TranslationHelpers';
import { getSharedWorkflowIds } from '@/WorkflowHelpers';

import { nodesController } from '@/api/nodes.api';
Expand Down Expand Up @@ -88,6 +87,7 @@ import {
MeController,
OwnerController,
PasswordResetController,
TranslationController,
UsersController,
} from '@/controllers';

Expand Down Expand Up @@ -366,6 +366,7 @@ class Server extends AbstractServer {
new OwnerController({ config, internalHooks, repositories, logger }),
new MeController({ externalHooks, internalHooks, repositories, logger }),
new PasswordResetController({ config, externalHooks, internalHooks, repositories, logger }),
new TranslationController(config, this.credentialTypes),
new UsersController({
config,
mailer,
Expand Down Expand Up @@ -606,48 +607,6 @@ class Server extends AbstractServer {
),
);

this.app.get(
`/${this.restEndpoint}/credential-translation`,
ResponseHelper.send(
async (
req: express.Request & { query: { credentialType: string } },
res: express.Response,
): Promise<object | null> => {
const translationPath = getCredentialTranslationPath({
locale: this.frontendSettings.defaultLocale,
credentialType: req.query.credentialType,
});

try {
return require(translationPath);
} catch (error) {
return null;
}
},
),
);

// Returns node information based on node names and versions
const headersPath = pathJoin(NODES_BASE_DIR, 'dist', 'nodes', 'headers');
this.app.get(
`/${this.restEndpoint}/node-translation-headers`,
ResponseHelper.send(
async (req: express.Request, res: express.Response): Promise<object | void> => {
try {
await fsAccess(`${headersPath}.js`);
} catch (_) {
return; // no headers available
}

try {
return require(headersPath);
} catch (error) {
res.status(500).send('Failed to load headers file');
}
},
),
);

// ----------------------------------------
// Node-Types
// ----------------------------------------
Expand Down
16 changes: 0 additions & 16 deletions packages/cli/src/TranslationHelpers.ts
@@ -1,7 +1,6 @@
import { join, dirname } from 'path';
import { readdir } from 'fs/promises';
import type { Dirent } from 'fs';
import { NODES_BASE_DIR } from '@/constants';

const ALLOWED_VERSIONED_DIRNAME_LENGTH = [2, 3]; // e.g. v1, v10

Expand Down Expand Up @@ -47,18 +46,3 @@ export async function getNodeTranslationPath({
? join(nodeDir, `v${maxVersion}`, 'translations', locale, `${nodeType}.json`)
: join(nodeDir, 'translations', locale, `${nodeType}.json`);
}

/**
* Get the full path to a credential translation file in `/dist`.
*/
export function getCredentialTranslationPath({
locale,
credentialType,
}: {
locale: string;
credentialType: string;
}): string {
const credsPath = join(NODES_BASE_DIR, 'dist', 'credentials');

return join(credsPath, 'translations', locale, `${credentialType}.json`);
}
1 change: 1 addition & 0 deletions packages/cli/src/controllers/index.ts
Expand Up @@ -2,4 +2,5 @@ export { AuthController } from './auth.controller';
export { MeController } from './me.controller';
export { OwnerController } from './owner.controller';
export { PasswordResetController } from './passwordReset.controller';
export { TranslationController } from './translation.controller';
export { UsersController } from './users.controller';
58 changes: 58 additions & 0 deletions packages/cli/src/controllers/translation.controller.ts
@@ -0,0 +1,58 @@
import type { Request } from 'express';
import { ICredentialTypes } from 'n8n-workflow';
import { join } from 'path';
import { access } from 'fs/promises';
import { Get, RestController } from '@/decorators';
import { BadRequestError, InternalServerError } from '@/ResponseHelper';
import { Config } from '@/config';
import { NODES_BASE_DIR } from '@/constants';

export const CREDENTIAL_TRANSLATIONS_DIR = 'n8n-nodes-base/dist/credentials/translations';
export const NODE_HEADERS_PATH = join(NODES_BASE_DIR, 'dist/nodes/headers');

export declare namespace TranslationRequest {
export type Credential = Request<{}, {}, {}, { credentialType: string }>;
}

@RestController('/')
export class TranslationController {
constructor(private config: Config, private credentialTypes: ICredentialTypes) {}

@Get('/credential-translation')
async getCredentialTranslation(req: TranslationRequest.Credential) {
const { credentialType } = req.query;

if (!this.credentialTypes.recognizes(credentialType))
throw new BadRequestError(`Invalid Credential type: "${credentialType}"`);

const defaultLocale = this.config.getEnv('defaultLocale');
const translationPath = join(
CREDENTIAL_TRANSLATIONS_DIR,
defaultLocale,
`${credentialType}.json`,
);

try {
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
return require(translationPath);
} catch (error) {
return null;
}
}

@Get('/node-translation-headers')
async getNodeTranslationHeaders() {
try {
await access(`${NODE_HEADERS_PATH}.js`);
} catch (_) {
return; // no headers available
}

try {
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
return require(NODE_HEADERS_PATH);
} catch (error) {
throw new InternalServerError('Failed to load headers file');
}
}
}
2 changes: 2 additions & 0 deletions packages/cli/test/setup-mocks.ts
@@ -1,3 +1,5 @@
import 'reflect-metadata';

jest.mock('@sentry/node');
jest.mock('@n8n_io/license-sdk');
jest.mock('@/telemetry');
Expand Down
40 changes: 40 additions & 0 deletions packages/cli/test/unit/controllers/translation.controller.test.ts
@@ -0,0 +1,40 @@
import { mock } from 'jest-mock-extended';
import type { ICredentialTypes } from 'n8n-workflow';
import type { Config } from '@/config';
import {
TranslationController,
TranslationRequest,
CREDENTIAL_TRANSLATIONS_DIR,
} from '@/controllers/translation.controller';
import { BadRequestError } from '@/ResponseHelper';

describe('TranslationController', () => {
const config = mock<Config>();
const credentialTypes = mock<ICredentialTypes>();
const controller = new TranslationController(config, credentialTypes);

describe('getCredentialTranslation', () => {
it('should throw 400 on invalid credential types', async () => {
const credentialType = 'not-a-valid-credential-type';
const req = mock<TranslationRequest.Credential>({ query: { credentialType } });
credentialTypes.recognizes.calledWith(credentialType).mockReturnValue(false);

expect(controller.getCredentialTranslation(req)).rejects.toThrowError(
new BadRequestError(`Invalid Credential type: "${credentialType}"`),
);
});

it('should return translation json on valid credential types', async () => {
const credentialType = 'credential-type';
const req = mock<TranslationRequest.Credential>({ query: { credentialType } });
config.getEnv.calledWith('defaultLocale').mockReturnValue('de');
credentialTypes.recognizes.calledWith(credentialType).mockReturnValue(true);
const response = { translation: 'string' };
jest.mock(`${CREDENTIAL_TRANSLATIONS_DIR}/de/credential-type.json`, () => response, {
virtual: true,
});

expect(await controller.getCredentialTranslation(req)).toEqual(response);
});
});
});

0 comments on commit f0f8d59

Please sign in to comment.