Skip to content
This repository was archived by the owner on Feb 2, 2021. It is now read-only.

Commit cef91c4

Browse files
author
Dimitar Tachev
authored
Merge pull request #1102 from telerik/tachev/fix-password-regex
fix: fix password regex and avoid infinite backtracking on bigger strings
2 parents 2bdc7a2 + 3be2a17 commit cef91c4

File tree

4 files changed

+41
-35
lines changed

4 files changed

+41
-35
lines changed

‎logger.ts

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,9 @@ const chalk = require("chalk");
77

88
export class Logger implements ILogger {
99
private log4jsLogger: log4js.ILogger = null;
10-
private encodeRequestPaths: string[] = ['/appbuilder/api/itmstransporter/applications?username='];
11-
private encodeBody: boolean = false;
12-
private passwordRegex = /(password=).*?(['&,]|$)|(["']?.*?password["']?\s*:\s*["']).*?(["'])/i;
10+
private passwordRegex = /(password=).*?(['&,]|$)|(password["']?\s*:\s*["']).*?(["'])/i;
1311
private passwordReplacement = "$1$3*******$2$4";
14-
private passwordBodyReplacement = "$1*******$2";
1512
private static LABEL = "[WARNING]:";
16-
private requestBodyRegex = /(^\").*?(\"$)/;
1713

1814
constructor($config: Config.IConfig,
1915
private $options: ICommonOptions) {
@@ -151,23 +147,10 @@ export class Logger implements ILogger {
151147

152148
private getPasswordEncodedArguments(args: string[]): string[] {
153149
return _.map(args, argument => {
154-
if (typeof argument !== 'string') {
155-
return argument;
150+
if (typeof argument === 'string' && !!argument.match(/password/i)) {
151+
argument = argument.replace(this.passwordRegex, this.passwordReplacement);
156152
}
157153

158-
argument = argument.replace(this.passwordRegex, this.passwordReplacement);
159-
160-
if (this.encodeBody) {
161-
argument = argument.replace(this.requestBodyRegex, this.passwordBodyReplacement);
162-
}
163-
164-
_.each(this.encodeRequestPaths, path => {
165-
if (argument.indexOf('path') > -1) {
166-
this.encodeBody = argument.indexOf(path) > -1;
167-
return false;
168-
}
169-
});
170-
171154
return argument;
172155
});
173156
}

‎test/unit-tests/logger.ts

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { Yok } from "../../yok";
22
import { Logger } from "../../logger";
3-
import * as assert from "assert";
3+
import * as path from "path";
4+
import { assert } from "chai";
5+
import * as fileSystemFile from "../../file-system";
46

57
const passwordReplacement = "*******";
68
const debugTrace = ["debug", "trace"];
@@ -14,6 +16,7 @@ function createTestInjector(logLevel?: string): IInjector {
1416
log: logLevel
1517
});
1618
testInjector.register("logger", Logger);
19+
testInjector.register("fs", fileSystemFile.FileSystem);
1720

1821
return testInjector;
1922
}
@@ -22,10 +25,12 @@ describe("logger", () => {
2225
let testInjector: IInjector;
2326
let logger: any;
2427
let outputs: any;
28+
let fs: IFileSystem;
2529

2630
beforeEach(() => {
2731
testInjector = createTestInjector();
2832
logger = testInjector.resolve("logger");
33+
fs = testInjector.resolve("fs");
2934
outputs = {
3035
debug: "",
3136
trace: ""
@@ -45,67 +50,83 @@ describe("logger", () => {
4550

4651
describe(debugTrace.join("+"), () => {
4752
_.each(debugTrace, methodName => {
53+
54+
it(`${methodName} should obfuscate password parameter when the string is larger`, () => {
55+
const dataFilePath = path.join(__dirname, './mocks/nativescript-cloud-npmjs-result.txt');
56+
const data = fs.readText(dataFilePath);
57+
const before = Date.now();
58+
logger[methodName].call(logger, data);
59+
const after = Date.now();
60+
console.log(after - before);
61+
62+
assert.notEqual(outputs[methodName].indexOf("password:'*******'"), -1);
63+
assert.isTrue(after - before < 10);
64+
});
65+
66+
it(`${methodName} should not get slower when the string is really large`, () => {
67+
const dataFilePath = path.join(__dirname, './mocks/tns-android-npmjs-result.txt');
68+
const data = fs.readText(dataFilePath);
69+
const before = Date.now();
70+
logger[methodName].call(logger, data);
71+
const after = Date.now();
72+
73+
assert.notEqual(outputs[methodName].indexOf("https://github.com/NativeScript/android-runtime"), -1);
74+
assert.isTrue(after - before < 10);
75+
});
76+
4877
_.each(passwordPair, passwordString => {
4978
it(`${methodName} should obfuscate properties ending in '${passwordString}' with values surrounded by single quotes`, () => {
5079
const logArgument = `{ certificate${passwordString}: 'pass', otherProperty: 'pass' }`;
5180

5281
logger[methodName].call(logger, logArgument);
5382

54-
assert.deepStrictEqual(outputs[methodName], `{ certificate${passwordString}: '${passwordReplacement}', otherProperty: 'pass' }`, `logger.${methodName} should obfuscate ${passwordString} properties`);
83+
assert.deepEqual(outputs[methodName], `{ certificate${passwordString}: '${passwordReplacement}', otherProperty: 'pass' }`, `logger.${methodName} should obfuscate ${passwordString} properties`);
5584
});
5685

5786
it(`${methodName} should obfuscate properties ending in '${passwordString}' with values surrounded by single quotes when it is the last property`, () => {
5887
const logArgument = `{ certificate${passwordString}: 'pass' }`;
5988

6089
logger[methodName].call(logger, logArgument);
6190

62-
assert.deepStrictEqual(outputs[methodName], `{ certificate${passwordString}: '${passwordReplacement}' }`, `logger.${methodName} should obfuscate ${passwordString} properties`);
91+
assert.deepEqual(outputs[methodName], `{ certificate${passwordString}: '${passwordReplacement}' }`, `logger.${methodName} should obfuscate ${passwordString} properties`);
6392
});
6493

6594
it(`${methodName} should obfuscate properties ending in '${passwordString}' with values surrounded by double quotes`, () => {
6695
const logArgument = `{ certificate${passwordString}: "pass", otherProperty: "pass" }`;
6796

6897
logger[methodName].call(logger, logArgument);
6998

70-
assert.deepStrictEqual(outputs[methodName], `{ certificate${passwordString}: "${passwordReplacement}", otherProperty: "pass" }`, `logger.${methodName} should obfuscate ${passwordString} properties`);
99+
assert.deepEqual(outputs[methodName], `{ certificate${passwordString}: "${passwordReplacement}", otherProperty: "pass" }`, `logger.${methodName} should obfuscate ${passwordString} properties`);
71100
});
72101

73102
it(`${methodName} should obfuscate properties ending in '${passwordString}' with values surrounded by double quotes when it is the last property`, () => {
74103
const logArgument = `{ certificate${passwordString}: "pass" }`;
75104

76105
logger[methodName].call(logger, logArgument);
77106

78-
assert.deepStrictEqual(outputs[methodName], `{ certificate${passwordString}: "${passwordReplacement}" }`, `logger.${methodName} should obfuscate ${passwordString} properties`);
107+
assert.deepEqual(outputs[methodName], `{ certificate${passwordString}: "${passwordReplacement}" }`, `logger.${methodName} should obfuscate ${passwordString} properties`);
79108
});
80109

81110
it(`${methodName} should obfuscate '${passwordString}' query parameter when it is the last query parameter`, () => {
82111
const logArgument = `{ proto: 'https', host: 'platform.telerik.com', path: '/appbuilder/api/itmstransporter/applications?username=dragon.telerikov%40yahoo.com&${passwordString}=somePassword', method: 'POST' }`;
83112

84113
logger[methodName].call(logger, logArgument);
85114

86-
assert.deepStrictEqual(outputs[methodName], `{ proto: 'https', host: 'platform.telerik.com', path: '/appbuilder/api/itmstransporter/applications?username=dragon.telerikov%40yahoo.com&${passwordString}=${passwordReplacement}', method: 'POST' }`, `logger.${methodName} should obfuscate ${passwordString} when in query parameter`);
115+
assert.deepEqual(outputs[methodName], `{ proto: 'https', host: 'platform.telerik.com', path: '/appbuilder/api/itmstransporter/applications?username=dragon.telerikov%40yahoo.com&${passwordString}=${passwordReplacement}', method: 'POST' }`, `logger.${methodName} should obfuscate ${passwordString} when in query parameter`);
87116
});
88117

89118
it(`${methodName} should obfuscate '${passwordString}' query parameter when it is not the last query parameter`, () => {
90119
const logArgument = `{ proto: 'https', host: 'platform.telerik.com', path: '/appbuilder/api/itmstransporter/applications?username=dragon.telerikov%40yahoo.com&${passwordString}=somePassword&data=someOtherData', method: 'POST' }`;
91120

92121
logger[methodName].call(logger, logArgument);
93122

94-
assert.deepStrictEqual(outputs[methodName], `{ proto: 'https', host: 'platform.telerik.com', path: '/appbuilder/api/itmstransporter/applications?username=dragon.telerikov%40yahoo.com&${passwordString}=${passwordReplacement}&data=someOtherData', method: 'POST' }`, `logger.${methodName} should obfuscate ${passwordString} when in query parameter`);
123+
assert.deepEqual(outputs[methodName], `{ proto: 'https', host: 'platform.telerik.com', path: '/appbuilder/api/itmstransporter/applications?username=dragon.telerikov%40yahoo.com&${passwordString}=${passwordReplacement}&data=someOtherData', method: 'POST' }`, `logger.${methodName} should obfuscate ${passwordString} when in query parameter`);
95124
});
96125
});
97126
});
98127
});
99128

100129
describe("trace", () => {
101-
it("should obfuscate body of request to /api/itmstransporter", () => {
102-
const request = "{ proto: 'https', host: 'platform.telerik.com', path: '/appbuilder/api/itmstransporter/applications?username=dragon.telerikov%40yahoo.com', method: 'POST' }";
103-
const requestBody = '"password"';
104-
105-
logger.trace(request, requestBody);
106-
107-
assert.deepEqual(outputs.trace, `${request}"${passwordReplacement}"`, "logger.trace should obfuscate body of api/itmstransporter requests");
108-
});
109130

110131
it("should not obfuscate body of other requests", () => {
111132
const request = "{ proto: 'https', host: 'platform.telerik.com', path: '/appbuilder/api/endpoint/applications?data=somedata, method: 'POST' }";

‎test/unit-tests/mocks/nativescript-cloud-npmjs-result.txt

Lines changed: 1 addition & 0 deletions
Large diffs are not rendered by default.

‎test/unit-tests/mocks/tns-android-npmjs-result.txt

Lines changed: 1 addition & 0 deletions
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)