From 9fa06f0e2a91e897a8f1d93f3dca6a732c3affdf Mon Sep 17 00:00:00 2001 From: Artem Derevnjuk Date: Mon, 30 May 2022 20:25:01 +0400 Subject: [PATCH 1/5] feat(scan): add a link to UI --- packages/scan/src/DefaultScans.ts | 9 +++++++-- packages/scan/src/commands/ListIssues.ts | 2 +- packages/scan/src/models/Issue.ts | 1 + 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/scan/src/DefaultScans.ts b/packages/scan/src/DefaultScans.ts index fe9e4df4..23614d5a 100644 --- a/packages/scan/src/DefaultScans.ts +++ b/packages/scan/src/DefaultScans.ts @@ -36,8 +36,13 @@ export class DefaultScans implements Scans { ); } - public listIssues(id: string): Promise { - return this.sendCommand(new ListIssues(id)); + public async listIssues(id: string): Promise { + const issues = await this.sendCommand(new ListIssues(id)); + + return issues.map(x => ({ + ...x, + link: `${this.configuration.api}/scans/${id}/issues/${x.id}` + })); } public async stopScan(id: string): Promise { diff --git a/packages/scan/src/commands/ListIssues.ts b/packages/scan/src/commands/ListIssues.ts index 4a0a75f4..e0bc3e44 100644 --- a/packages/scan/src/commands/ListIssues.ts +++ b/packages/scan/src/commands/ListIssues.ts @@ -1,7 +1,7 @@ import { Issue } from '../models'; import { HttpRequest } from '@sec-tester/bus'; -export class ListIssues extends HttpRequest { +export class ListIssues extends HttpRequest[]> { constructor(id: string) { super({ url: `/api/v1/scans/${id}/issues`, diff --git a/packages/scan/src/models/Issue.ts b/packages/scan/src/models/Issue.ts index 90ef36a9..c9c03530 100644 --- a/packages/scan/src/models/Issue.ts +++ b/packages/scan/src/models/Issue.ts @@ -49,6 +49,7 @@ export interface Issue { time: Date; originalRequest: Request; request: Request; + link: string; exposure?: string; resources?: string[]; comments?: Comment[]; From b6affe8edb6157edf01117fa7c6b289fbf4f4c56 Mon Sep 17 00:00:00 2001 From: Artem Derevnjuk Date: Tue, 31 May 2022 11:08:52 +0400 Subject: [PATCH 2/5] feat(reporter): introduce issue formatters --- packages/reporter/README.md | 44 +++++++++- packages/reporter/src/__fixtures__/issues.ts | 63 ++++++++++++++ .../src/formatters/PlainTextFormatter.spec.ts | 44 ++++++++++ .../src/formatters/PlainTextFormatter.ts | 84 +++++++++++++++++++ packages/reporter/src/formatters/index.ts | 1 + packages/reporter/src/index.ts | 3 +- packages/reporter/src/lib/Formatter.ts | 7 ++ packages/reporter/src/lib/index.ts | 3 +- 8 files changed, 246 insertions(+), 3 deletions(-) create mode 100644 packages/reporter/src/__fixtures__/issues.ts create mode 100644 packages/reporter/src/formatters/PlainTextFormatter.spec.ts create mode 100644 packages/reporter/src/formatters/PlainTextFormatter.ts create mode 100644 packages/reporter/src/formatters/index.ts create mode 100644 packages/reporter/src/lib/Formatter.ts diff --git a/packages/reporter/README.md b/packages/reporter/README.md index e7b1f339..c1ef4888 100644 --- a/packages/reporter/README.md +++ b/packages/reporter/README.md @@ -15,7 +15,7 @@ npm i -s @sec-tester/reporter ## Usage -The package provides only one implementation of the `Reporter` that lets to get results to stdout, i.e. `StdReporter`: +The package provides an implementation of the `Reporter` that lets to get results to stdout, i.e. `StdReporter`: ```ts import { Reporter, StdReporter } from '@sec-tester/reporter'; @@ -36,6 +36,48 @@ await reporter.report(scan); +In addition, the package exposes a `PlainTextFormatter` that implements a `Formatter` interface: + +```ts +import { Formatter, PlainTextFormatter } from '@sec-tester/reporter'; + +const formatter: Formatter = new PlainTextFormatter(); +``` + +To convert an issue into text, you just need to call the `format` method: + +```ts +formatter.format(issue); +``` + +
+Sample output + +``` +Issue in Bright UI: https://app.neuralegion.com/scans/djoqtSDRJYaR6sH8pfYpDX/issues/8iacauN1FH9vFvDCLoo42v +Name: Missing Strict-Transport-Security Header +Severity: Low +Remediation: +Make sure to proprely set and configure headers on your application - missing strict-transport-security header +Details: +The engine detected a missing strict-transport-security header. Headers are used to outline communication and +improve security of application. +Extra Details: +● Missing Strict-Transport-Security Header + The engine detected a missing Strict-Transport-Security header, which might cause data to be sent insecurely from the client to the server. + Remedy: + - Make sure to set this header to one of the following options: + 1. Strict-Transport-Security: max-age= + 2. Strict-Transport-Security: max-age=; includeSubDomains + 3. Strict-Transport-Security: max-age=; preload + Resources: + - https://www.owasp.org/index.php/OWASP_Secure_Headers_Project#hsts + Issues found on the following URLs: + - [GET] https://qa.brokencrystals.com/ +``` + +
+ ## License Copyright © 2022 [Bright Security](https://brightsec.com/). diff --git a/packages/reporter/src/__fixtures__/issues.ts b/packages/reporter/src/__fixtures__/issues.ts new file mode 100644 index 00000000..c7c4599b --- /dev/null +++ b/packages/reporter/src/__fixtures__/issues.ts @@ -0,0 +1,63 @@ +import { HttpMethod, Severity } from '@sec-tester/scan'; + +export const issueWithoutResourcesText = `Issue in Bright UI: http://app.neuralegion.com/scans/pDzxcEXQC8df1fcz1QwPf9/issues/pDzxcEXQC8df1fcz1QwPf9 +Name: Database connection crashed +Severity: Medium +Remediation: +The best way to protect against those kind of issues is making sure the Database resources are sufficient +Details: +Cross-site request forgery is a type of malicious website exploit.`; +export const issueWithoutResources = { + id: 'pDzxcEXQC8df1fcz1QwPf9', + order: 1, + details: 'Cross-site request forgery is a type of malicious website exploit.', + name: 'Database connection crashed', + severity: Severity.MEDIUM, + protocol: 'http', + remedy: + 'The best way to protect against those kind of issues is making sure the Database resources are sufficient', + cvss: 'CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:L', + time: new Date(), + originalRequest: { + method: HttpMethod.GET, + url: 'https://brokencrystals.com/' + }, + request: { + method: HttpMethod.GET, + url: 'https://brokencrystals.com/' + }, + link: 'http://app.neuralegion.com/scans/pDzxcEXQC8df1fcz1QwPf9/issues/pDzxcEXQC8df1fcz1QwPf9' +}; + +export const fullyDescribedIssueText = `${issueWithoutResourcesText} +Extra Details: +● Missing Strict-Transport-Security Header +\tThe engine detected a missing Strict-Transport-Security header, which might cause data to be sent insecurely from the client to the server. +\tLinks: +\t● https://www.owasp.org/index.php/OWASP_Secure_Headers_Project#hsts +References: +● https://www.owasp.org/index.php/OWASP_Secure_Headers_Project#hsts`; +export const fullyDescribedIssue = { + ...issueWithoutResources, + comments: [ + { + headline: 'Missing Strict-Transport-Security Header', + text: 'The engine detected a missing Strict-Transport-Security header, which might cause data to be sent insecurely from the client to the server.', + links: [ + 'https://www.owasp.org/index.php/OWASP_Secure_Headers_Project#hsts' + ] + } + ], + resources: [ + 'https://www.owasp.org/index.php/OWASP_Secure_Headers_Project#hsts' + ] +}; +export const issueWithoutExtraInfoText = `${issueWithoutResourcesText} +References: + ● https://www.owasp.org/index.php/OWASP_Secure_Headers_Project#hsts`; +export const issueWithoutExtraInfo = { + ...issueWithoutResources, + resources: [ + 'https://www.owasp.org/index.php/OWASP_Secure_Headers_Project#hsts' + ] +}; diff --git a/packages/reporter/src/formatters/PlainTextFormatter.spec.ts b/packages/reporter/src/formatters/PlainTextFormatter.spec.ts new file mode 100644 index 00000000..c678420a --- /dev/null +++ b/packages/reporter/src/formatters/PlainTextFormatter.spec.ts @@ -0,0 +1,44 @@ +import { PlainTextFormatter } from './PlainTextFormatter'; +import { + fullyDescribedIssue, + fullyDescribedIssueText, + issueWithoutExtraInfo, + issueWithoutExtraInfoText, + issueWithoutResources, + issueWithoutResourcesText +} from '../__fixtures__/issues'; +import { Issue } from '@sec-tester/scan'; + +describe('PlainTextFormatter', () => { + let formatter!: PlainTextFormatter; + + beforeEach(() => { + formatter = new PlainTextFormatter(); + }); + + describe('format', () => { + it.each([ + { + input: fullyDescribedIssue, + expected: fullyDescribedIssueText, + title: 'fully described issue' + }, + { + input: issueWithoutExtraInfo, + expected: issueWithoutExtraInfoText, + title: 'issue without extra info' + }, + { + input: issueWithoutResources, + expected: issueWithoutResourcesText, + title: 'issue without resources' + } + ])('should format $title', ({ input, expected }) => { + // act + const result = formatter.format(input as Issue); + + // assert + expect(result).toEqual(expected); + }); + }); +}); diff --git a/packages/reporter/src/formatters/PlainTextFormatter.ts b/packages/reporter/src/formatters/PlainTextFormatter.ts new file mode 100644 index 00000000..64400b3b --- /dev/null +++ b/packages/reporter/src/formatters/PlainTextFormatter.ts @@ -0,0 +1,84 @@ +import { Formatter } from '../lib'; +import { Comment, Issue } from '@sec-tester/scan'; +import { format } from 'util'; + +export class PlainTextFormatter implements Formatter { + private readonly BULLET_POINT = '●'; + private readonly NEW_LINE = '\n'; + private readonly TABULATION = '\t'; + + public format(issue: Issue): string { + const { + link, + name, + severity, + remedy, + details, + comments = [], + resources = [] + } = issue; + const template = this.generateTemplate({ + extraInfo: comments.length > 0, + references: resources.length > 0 + }); + + const message = format( + template, + link, + name, + severity, + remedy, + details, + this.formatList(comments, comment => this.formatExtraInfo(comment)), + this.formatList(resources) + ); + + return message.trim(); + } + + private generateTemplate(options: { + extraInfo: boolean; + references: boolean; + }): string { + return ` +Issue in Bright UI: %s +Name: %s +Severity: %s +Remediation: +%s +Details: +%s${options.extraInfo ? `\nExtra Details:\n%s` : ''}${ + options.references ? `\nReferences:\n%s` : '' + }`.trim(); + } + + private formatExtraInfo({ headline, text = '', links = [] }: Comment) { + const footer = links.length + ? this.combineList(['Links:', this.formatList(links)]) + : ''; + const blocks = [text, footer].map(x => this.indent(x)); + const document = this.combineList(blocks); + + return this.combineList([headline, document]); + } + + private indent(x: string, length: number = 1) { + const lines = x.split(this.NEW_LINE); + + return this.combineList( + lines.map(line => `${this.TABULATION.repeat(length)}${line}`) + ); + } + + private formatList(list: T[], map?: (x: T) => string): string { + const items = list.map( + x => `${this.BULLET_POINT} ${typeof map == 'function' ? map(x) : x}` + ); + + return this.combineList(items); + } + + private combineList(list: string[], sep?: string): string { + return list.join(sep ?? this.NEW_LINE); + } +} diff --git a/packages/reporter/src/formatters/index.ts b/packages/reporter/src/formatters/index.ts new file mode 100644 index 00000000..6ff8f48d --- /dev/null +++ b/packages/reporter/src/formatters/index.ts @@ -0,0 +1 @@ +export * from './PlainTextFormatter'; diff --git a/packages/reporter/src/index.ts b/packages/reporter/src/index.ts index 9eaed5cb..ff4dbf37 100644 --- a/packages/reporter/src/index.ts +++ b/packages/reporter/src/index.ts @@ -1,2 +1,3 @@ -export { Reporter } from './lib'; +export { PlainTextFormatter } from './formatters'; +export { Reporter, Formatter } from './lib'; export { StdReporter } from './std'; diff --git a/packages/reporter/src/lib/Formatter.ts b/packages/reporter/src/lib/Formatter.ts new file mode 100644 index 00000000..086e2485 --- /dev/null +++ b/packages/reporter/src/lib/Formatter.ts @@ -0,0 +1,7 @@ +import { Issue } from '@sec-tester/scan'; + +export interface Formatter { + format(issue: Issue): string; +} + +export const Formatter: unique symbol = Symbol('Formatter'); diff --git a/packages/reporter/src/lib/index.ts b/packages/reporter/src/lib/index.ts index 3bde4574..2e572fbc 100644 --- a/packages/reporter/src/lib/index.ts +++ b/packages/reporter/src/lib/index.ts @@ -1 +1,2 @@ -export { Reporter } from './Reporter'; +export * from './Reporter'; +export * from './Formatter'; From 1be64f2972cd13b21352f7cac5e9f9d6bd5e7852 Mon Sep 17 00:00:00 2001 From: Artem Derevnjuk Date: Tue, 31 May 2022 13:24:39 +0400 Subject: [PATCH 3/5] feat(runner): add more info in an error message --- packages/runner/src/lib/IssueFound.ts | 9 +++++++++ packages/runner/src/lib/SecRunner.ts | 8 ++++---- packages/runner/src/lib/SecScan.spec.ts | 24 +++++++++++++----------- packages/runner/src/lib/SecScan.ts | 23 +++++++++++++++-------- 4 files changed, 41 insertions(+), 23 deletions(-) create mode 100644 packages/runner/src/lib/IssueFound.ts diff --git a/packages/runner/src/lib/IssueFound.ts b/packages/runner/src/lib/IssueFound.ts new file mode 100644 index 00000000..008cfcce --- /dev/null +++ b/packages/runner/src/lib/IssueFound.ts @@ -0,0 +1,9 @@ +import { Formatter } from '@sec-tester/reporter'; +import { SecTesterError } from '@sec-tester/core'; +import { Issue } from '@sec-tester/scan'; + +export class IssueFound extends SecTesterError { + constructor(public readonly issue: Issue, formatter: Formatter) { + super(`Target is vulnerable\n\n${formatter.format(issue)}`); + } +} diff --git a/packages/runner/src/lib/SecRunner.ts b/packages/runner/src/lib/SecRunner.ts index 63ae2ae4..c4698565 100644 --- a/packages/runner/src/lib/SecRunner.ts +++ b/packages/runner/src/lib/SecRunner.ts @@ -6,8 +6,8 @@ import { RepeaterFactory, RepeatersManager } from '@sec-tester/repeater'; -import { Reporter, StdReporter } from '@sec-tester/reporter'; import { ScanFactory } from '@sec-tester/scan'; +import { Formatter, PlainTextFormatter } from '@sec-tester/reporter'; export class SecRunner { private readonly configuration: Configuration; @@ -64,15 +64,15 @@ export class SecRunner { repeaterId: this.repeater.repeaterId }, this.configuration.container.resolve(ScanFactory), - this.configuration.container.resolve(Reporter) + this.configuration.container.resolve(Formatter) ); } private async initConfiguration(configuration: Configuration): Promise { await configuration.loadCredentials(); - configuration.container.register(Reporter, { - useClass: StdReporter + configuration.container.register(Formatter, { + useClass: PlainTextFormatter }); } } diff --git a/packages/runner/src/lib/SecScan.spec.ts b/packages/runner/src/lib/SecScan.spec.ts index 37a79b97..144cce59 100644 --- a/packages/runner/src/lib/SecScan.spec.ts +++ b/packages/runner/src/lib/SecScan.spec.ts @@ -1,5 +1,6 @@ import { SecScan } from './SecScan'; import { resolvableInstance } from './SecRunner.spec'; +import { IssueFound } from './IssueFound'; import { Issue, Scan, @@ -20,7 +21,7 @@ import { } from 'ts-mockito'; import { DependencyContainer } from 'tsyringe'; import { Configuration } from '@sec-tester/core'; -import { Reporter } from '@sec-tester/reporter'; +import { Formatter } from '@sec-tester/reporter'; describe('SecScan', () => { const tests = [TestType.XSS]; @@ -29,9 +30,9 @@ describe('SecScan', () => { const mockedConfiguration = mock(); const mockedScanFactory = mock(); const mockedScan = mock(); - const mockedReporter = mock(); + const mockedIssueFormatter = mock(); - const reporter = instance(mockedReporter); + const issueFormatter = instance(mockedIssueFormatter); const scanFactory = instance(mockedScanFactory); beforeEach(() => { @@ -43,19 +44,19 @@ describe('SecScan', () => { }); afterEach(() => { - reset( + reset( mockedContainer, mockedConfiguration, mockedScanFactory, mockedScan, - mockedReporter + mockedIssueFormatter ); }); describe('constructor', () => { it('should create instance', () => { expect( - () => new SecScan({ tests }, scanFactory, reporter) + () => new SecScan({ tests }, scanFactory, issueFormatter) ).not.toThrowError(); }); }); @@ -64,14 +65,15 @@ describe('SecScan', () => { const target: TargetOptions = { url: 'http://foo.bar' }; const issues: Issue[] = [ { - id: 'fooId' + id: 'fooId', + severity: Severity.HIGH } as Issue ]; let secScan!: SecScan; beforeEach(() => { - secScan = new SecScan({ tests }, scanFactory, reporter); + secScan = new SecScan({ tests }, scanFactory, issueFormatter); }); it('should run scan with default threshold', async () => { @@ -121,7 +123,7 @@ describe('SecScan', () => { const res = secScan.run(target); - await expect(res).rejects.toThrow('Target is vulnerable'); + await expect(res).rejects.toThrow(IssueFound); }); it('should stop scan after resolved expectation', async () => { @@ -149,7 +151,7 @@ describe('SecScan', () => { const res = secScan.run(target); await expect(res).rejects.toThrow(); - verify(mockedReporter.report(anything())).once(); + verify(mockedIssueFormatter.format(anything())).once(); }); it('should not report if there are not issues', async () => { @@ -158,7 +160,7 @@ describe('SecScan', () => { await secScan.run(target); - verify(mockedReporter.report(anything())).never(); + verify(mockedIssueFormatter.format(anything())).never(); }); }); }); diff --git a/packages/runner/src/lib/SecScan.ts b/packages/runner/src/lib/SecScan.ts index e5b15904..1816e418 100644 --- a/packages/runner/src/lib/SecScan.ts +++ b/packages/runner/src/lib/SecScan.ts @@ -1,9 +1,12 @@ -import { Reporter } from '@sec-tester/reporter'; +import { IssueFound } from './IssueFound'; +import { Formatter } from '@sec-tester/reporter'; import { + Issue, Scan, ScanFactory, ScanSettingsOptions, Severity, + severityRanges, TargetOptions } from '@sec-tester/scan'; @@ -14,7 +17,7 @@ export class SecScan { constructor( private readonly settings: Omit, private readonly scanFactory: ScanFactory, - private readonly reporter: Reporter + private readonly formatter: Formatter ) {} public async run(target: TargetOptions): Promise { @@ -50,16 +53,20 @@ export class SecScan { } private async assert(scan: Scan): Promise { - const issues = await scan.issues(); + const issue = await this.findExpectedIssue(scan); - if (issues.length) { - await this.report(scan); + if (issue) { + throw new IssueFound(issue, this.formatter); } } - private async report(scan: Scan): Promise { - await this.reporter.report(scan); + private async findExpectedIssue(scan: Scan): Promise { + const issues = await scan.issues(); - throw new Error('Target is vulnerable'); + if (this._threshold) { + return issues.find(x => + severityRanges.get(this._threshold)?.includes(x.severity) + ); + } } } From d4700851f01dd8f1b65343823e2ac72d0130445f Mon Sep 17 00:00:00 2001 From: Artem Derevnjuk Date: Tue, 31 May 2022 13:24:59 +0400 Subject: [PATCH 4/5] docs(readme): add link to the demo project --- README.md | 48 +----------------------------------------------- 1 file changed, 1 insertion(+), 47 deletions(-) diff --git a/README.md b/README.md index 34b20232..bc56e601 100644 --- a/README.md +++ b/README.md @@ -81,53 +81,7 @@ $ yarn add @sec-tester/runner \ ### Usage examples -Here is an example to check your own application for XSS vulnerabilities: - -```ts -import { SecRunner, SecScan } from '@sec-tester/runner'; -import { Severity, TestType } from '@sec-tester/scan'; - -describe('/api', () => { - let runner!: SecRunner; - let scan!: SecScan; - - beforeEach(async () => { - runner = new SecRunner({ hostname: 'app.neuralegion.com' }); - - await runner.init(); - - scan = runner - .createScan({ tests: [TestType.XSS] }) - .threshold(Severity.MEDIUM) // i. e. ignore LOW severity issues - .timeout(300000); // i. e. fail if last longer than 5 minutes - }); - - afterEach(async () => { - await runner.clear(); - }); - - describe('/orders', () => { - it('should not have persistent xss', async () => { - await scan.run({ - method: 'POST', - url: 'https://localhost:8000/api/orders', - body: { subject: 'Test', body: "" } - }); - }); - - it('should not have reflective xss', async () => { - await scan.run({ - url: 'https://localhost:8000/api/orders', - query: { - q: `` - } - }); - }); - }); -}); -``` - -Full documentation can be found in [**runner**](https://github.com/NeuraLegion/sec-tester-js/tree/master/packages/runner). +Full configuration & usage examples can be found in our [demo project](https://github.com/NeuraLegion/sec-tester-js-demo). ## Documentation & Help From 0b045eac7816c2ff15e2284eb252ba8f2e379dcc Mon Sep 17 00:00:00 2001 From: Artem Derevnjuk Date: Tue, 31 May 2022 13:25:53 +0400 Subject: [PATCH 5/5] test(scan): fix compile-time error --- packages/scan/src/DefaultScans.spec.ts | 8 +++++++- packages/scan/src/Scan.spec.ts | 3 ++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/scan/src/DefaultScans.spec.ts b/packages/scan/src/DefaultScans.spec.ts index 4846fc9b..1c418681 100644 --- a/packages/scan/src/DefaultScans.spec.ts +++ b/packages/scan/src/DefaultScans.spec.ts @@ -172,6 +172,12 @@ describe('HttpScans', () => { } } ]; + const api = 'https://localhost'; + const expected = issues.map(x => ({ + ...x, + link: `${api}/scans/${id}/issues/${x.id}` + })); + when(mockedConfiguration.api).thenReturn('https://localhost'); when(mockedCommandDispatcher.execute(anyOfClass(ListIssues))).thenResolve( issues ); @@ -179,7 +185,7 @@ describe('HttpScans', () => { const result = await scans.listIssues(id); verify(mockedCommandDispatcher.execute(anyOfClass(ListIssues))).once(); - expect(result).toEqual(issues); + expect(result).toEqual(expected); }); it('should raise an error if result is not defined', async () => { diff --git a/packages/scan/src/Scan.spec.ts b/packages/scan/src/Scan.spec.ts index 414b9c5e..7e9598bb 100644 --- a/packages/scan/src/Scan.spec.ts +++ b/packages/scan/src/Scan.spec.ts @@ -93,7 +93,8 @@ describe('Scan', () => { request: { method: HttpMethod.GET, url: 'https://brokencrystals.com/' - } + }, + link: 'https://app.neuralegion.com/scans/pDzxcEXQC8df1fcz1QwPf9/issues/pDzxcEXQC8df1fcz1QwPf9' }; when(mockedScans.listIssues(id)).thenResolve([issue]); when(mockedScans.getScan(id)).thenResolve({ status: ScanStatus.DONE });