diff --git a/packages/runner/src/lib/IssueFound.ts b/packages/runner/src/lib/IssueFound.ts index b68d07c6..e4ad9e63 100644 --- a/packages/runner/src/lib/IssueFound.ts +++ b/packages/runner/src/lib/IssueFound.ts @@ -3,7 +3,10 @@ import { SecTesterError } from '@sectester/core'; import { Issue } from '@sectester/scan'; export class IssueFound extends SecTesterError { - constructor(public readonly issue: Issue, formatter: Formatter) { + constructor( + public readonly issue: Issue, + formatter: Formatter + ) { super(`Target is vulnerable\n\n${formatter.format(issue)}`); } } diff --git a/packages/scan/src/DefaultScans.spec.ts b/packages/scan/src/DefaultScans.spec.ts index a82dab4f..1cec1ffa 100644 --- a/packages/scan/src/DefaultScans.spec.ts +++ b/packages/scan/src/DefaultScans.spec.ts @@ -25,11 +25,7 @@ describe('DefaultScans', () => { }); afterEach(() => - reset( - mockedApiClient, - mockedCi, - mockedConfiguration - ) + reset(mockedApiClient, mockedConfiguration) ); describe('createScan', () => { @@ -70,6 +66,168 @@ describe('DefaultScans', () => { expect(result).toEqual({ id }); }); + + it('should transform broken_access_control test with single auth object into backend compatible format', async () => { + const response = new Response(JSON.stringify({ id })); + + when(mockedConfiguration.name).thenReturn('test'); + when(mockedConfiguration.version).thenReturn('1.0'); + when(mockedCi.name).thenReturn('github'); + + when( + mockedApiClient.request( + '/api/v1/scans', + deepEqual({ + method: 'POST', + headers: { + 'content-type': 'application/json' + }, + body: JSON.stringify({ + projectId, + name: 'test', + entryPointIds: [entryPointId], + tests: ['xss', 'broken_access_control'], + testMetadata: { + broken_access_control: { + authObjectId: [null, 'auth-id-123'] + } + }, + info: { + source: 'utlib', + provider: 'github', + client: { + name: 'test', + version: '1.0' + } + } + }) + }) + ) + ).thenResolve(response); + + const result = await scans.createScan({ + projectId, + name: 'test', + entryPointIds: [entryPointId], + tests: [ + 'xss', + { + name: 'broken_access_control' as const, + options: { auth: 'auth-id-123' } + } + ] + }); + + expect(result).toEqual({ id }); + }); + + it('should transform broken_access_control test with two auth objects into backend compatible format', async () => { + const response = new Response(JSON.stringify({ id })); + const config = { + projectId, + name: 'test scan', + entryPointIds: [entryPointId], + tests: [ + { + name: 'broken_access_control' as const, + options: { auth: ['auth-id-1', 'auth-id-2'] as [string, string] } + } + ] + }; + + when(mockedConfiguration.name).thenReturn('test'); + when(mockedConfiguration.version).thenReturn('1.0'); + when(mockedCi.name).thenReturn('github'); + + when( + mockedApiClient.request( + '/api/v1/scans', + deepEqual({ + method: 'POST', + headers: { + 'content-type': 'application/json' + }, + body: JSON.stringify({ + projectId, + name: 'test scan', + entryPointIds: [entryPointId], + tests: ['broken_access_control'], + testMetadata: { + broken_access_control: { + authObjectId: ['auth-id-1', 'auth-id-2'] + } + }, + info: { + source: 'utlib', + provider: 'github', + client: { + name: 'test', + version: '1.0' + } + } + }) + }) + ) + ).thenResolve(response); + + const result = await scans.createScan(config); + + expect(result).toEqual({ id }); + }); + + it('should throw error when broken_access_control test has no auth option', async () => { + const config = { + projectId, + name: 'test scan', + entryPointIds: [entryPointId], + tests: [ + { + name: 'broken_access_control' as const, + options: {} as any + } + ] + }; + + await expect(scans.createScan(config)).rejects.toThrow( + 'Auth option is required for broken_access_control test' + ); + }); + + it('should throw error when broken_access_control test has empty auth array', async () => { + const config = { + projectId, + name: 'test scan', + entryPointIds: [entryPointId], + tests: [ + { + name: 'broken_access_control' as const, + options: { auth: [] as any } + } + ] + }; + + await expect(scans.createScan(config)).rejects.toThrow( + 'broken_access_control test auth option must be either a string or a tuple of two strings' + ); + }); + + it('should throw error when broken_access_control test has auth array with 3 elements', async () => { + const config = { + projectId, + name: 'test scan', + entryPointIds: [entryPointId], + tests: [ + { + name: 'broken_access_control' as const, + options: { auth: ['auth-id-1', 'auth-id-2', 'auth-id-3'] as any } + } + ] + }; + + await expect(scans.createScan(config)).rejects.toThrow( + 'broken_access_control test auth option must be either a string or a tuple of two strings' + ); + }); }); describe('listIssues', () => { diff --git a/packages/scan/src/DefaultScans.ts b/packages/scan/src/DefaultScans.ts index ae24e095..67cd0b3a 100644 --- a/packages/scan/src/DefaultScans.ts +++ b/packages/scan/src/DefaultScans.ts @@ -1,5 +1,6 @@ import { Scans } from './Scans'; import { Issue, ScanConfig, ScanState } from './models'; +import { BrokenAccessControlTest } from './models/Tests'; import { inject, injectable } from 'tsyringe'; import { ApiClient, ApiError, Configuration } from '@sectester/core'; import ci from 'ci-info'; @@ -19,7 +20,7 @@ export class DefaultScans implements Scans { 'content-type': 'application/json' }, body: JSON.stringify({ - ...config, + ...this.transformConfig(config), info: { source: 'utlib', provider: ci.name, @@ -80,4 +81,74 @@ export class DefaultScans implements Scans { return result; } + + private transformConfig(config: ScanConfig): Record { + if (!config.tests) { + return { ...config }; + } + + const { mappedTests, testMetadata } = this.mapTests(config.tests); + const { tests: originalTests, ...restConfig } = config; + + if (Object.keys(testMetadata).length > 0) { + const result: Record = { + ...restConfig, + tests: mappedTests, + testMetadata + }; + + return result; + } + + return { ...config }; + } + + private mapTests(tests: ScanConfig['tests']) { + const mappedTests: string[] = []; + const testMetadata: Record = {}; + + if (!tests) { + throw new Error('Scan config should have tests defined'); + } + + for (const test of tests) { + if (typeof test === 'string') { + mappedTests.push(test); + continue; + } + + if (test.name === 'broken_access_control') { + this.mapBrokenAccessControlTest(test, mappedTests, testMetadata); + } else { + throw new Error(`Unsupported configurable test: ${test.name}`); + } + } + + return { mappedTests, testMetadata }; + } + + private mapBrokenAccessControlTest( + test: BrokenAccessControlTest, + mappedTests: string[], + testMetadata: Record + ) { + if (!test.options?.auth) { + throw new Error('Auth option is required for broken_access_control test'); + } + + const { auth } = test.options; + if ( + typeof auth !== 'string' && + (!Array.isArray(auth) || auth.length !== 2) + ) { + throw new Error( + `${test.name} test auth option must be either a string or a tuple of two strings` + ); + } + + mappedTests.push(test.name); + testMetadata[test.name] = { + authObjectId: typeof auth === 'string' ? [null, auth] : [auth[0], auth[1]] + }; + } } diff --git a/packages/scan/src/ScanSettings.spec.ts b/packages/scan/src/ScanSettings.spec.ts index 31564665..66347e67 100644 --- a/packages/scan/src/ScanSettings.spec.ts +++ b/packages/scan/src/ScanSettings.spec.ts @@ -187,5 +187,106 @@ describe('ScanSettings', () => { name: expect.stringMatching(/^.{1,199}…$/) }); }); + + it('should handle broken access control test with string auth', () => { + // arrange + const testConfig = { + name: 'broken_access_control' as const, + options: { + auth: 'auth-object-id' + } + }; + const settings: ScanSettingsOptions = { + tests: [testConfig], + target: { url: 'https://example.com' } + }; + + // act + const result = new ScanSettings(settings); + + // assert + expect(result.tests).toEqual([testConfig]); + }); + + it('should handle broken access control test with tuple auth', () => { + // arrange + const testConfig = { + name: 'broken_access_control' as const, + options: { + auth: ['key', 'value'] as [string, string] + } + }; + const settings: ScanSettingsOptions = { + tests: [testConfig], + target: { url: 'https://example.com' } + }; + + // act + const result = new ScanSettings(settings); + + // assert + expect(result.tests).toEqual([testConfig]); + }); + + it('should deduplicate string tests', () => { + // arrange + const testName = 'xss'; + const settings: ScanSettingsOptions = { + tests: [testName, testName], + target: { url: 'https://example.com' } + }; + + // act + const result = new ScanSettings(settings); + + // assert + expect(result.tests).toEqual([testName]); + }); + + it('should not allow duplicated tests with options', () => { + // arrange + const testConfig1 = { + name: 'broken_access_control' as const, + options: { + auth: 'auth1' + } + }; + const testConfig2 = { + name: 'broken_access_control' as const, + options: { + auth: 'auth2' + } + }; + const settings: ScanSettingsOptions = { + tests: [testConfig1, testConfig2], + target: { url: 'https://example.com' } + }; + + // act & assert + expect(() => new ScanSettings(settings)).toThrow( + 'Duplicate test configuration found: broken_access_control' + ); + }); + + it('should handle mixed string and configurable tests', () => { + // arrange + const stringTest = 'xss'; + const testWithOptions = { + name: 'broken_access_control' as const, + options: { + auth: 'auth-object-id' + } + }; + const settings: ScanSettingsOptions = { + tests: [stringTest, testWithOptions], + target: { url: 'https://example.com' } + }; + + // act + const result = new ScanSettings(settings); + + // assert + expect(result.tests).toEqual([stringTest, testWithOptions]); + }); }); }); diff --git a/packages/scan/src/ScanSettings.ts b/packages/scan/src/ScanSettings.ts index 71ac1f4a..d60bf7b1 100644 --- a/packages/scan/src/ScanSettings.ts +++ b/packages/scan/src/ScanSettings.ts @@ -1,10 +1,10 @@ -import { AttackParamLocation, HttpMethod } from './models'; +import { AttackParamLocation, HttpMethod, Test } from './models'; import { Target, TargetOptions } from './target'; import { checkBoundaries, contains, truncate } from '@sectester/core'; export interface ScanSettingsOptions { // The list of tests to be performed against the target application - tests: string[]; + tests: Test[]; // The target that will be attacked target: Target | TargetOptions; // The scan name @@ -120,20 +120,37 @@ export class ScanSettings implements ScanSettingsOptions { this._requestsRateLimit = value; } - private _tests!: string[]; + private _tests!: Test[]; - get tests(): string[] { + get tests(): Test[] { return this._tests; } - private set tests(value: string[]) { - const uniqueTestTypes = new Set(value); - - if (uniqueTestTypes.size < 1) { + private set tests(value: Test[]) { + if (value.length < 1) { throw new Error('Please provide at least one test.'); } - this._tests = [...uniqueTestTypes]; + const simpleTests = new Set(); + const configurableTests: Test[] = []; + const seenTestConfigurations = new Set(); + + for (const test of value) { + const testName = typeof test === 'string' ? test : test.name; + + if (typeof test === 'string') { + simpleTests.add(test); + continue; + } + + if (seenTestConfigurations.has(testName) || simpleTests.has(testName)) { + throw new Error(`Duplicate test configuration found: ${testName}`); + } + seenTestConfigurations.add(testName); + configurableTests.push(test); + } + + this._tests = [...simpleTests, ...configurableTests]; } private _attackParamLocations!: AttackParamLocation[]; diff --git a/packages/scan/src/models/ScanConfig.ts b/packages/scan/src/models/ScanConfig.ts index 67d4204d..641130fe 100644 --- a/packages/scan/src/models/ScanConfig.ts +++ b/packages/scan/src/models/ScanConfig.ts @@ -1,10 +1,11 @@ import { AttackParamLocation } from './AttackParamLocation'; +import { Test } from './Tests'; export interface ScanConfig { name: string; projectId: string; entryPointIds: string[]; - tests?: string[]; + tests?: Test[]; poolSize?: number; requestsRateLimit?: number; attackParamLocations?: AttackParamLocation[]; diff --git a/packages/scan/src/models/Severity.spec.ts b/packages/scan/src/models/Severity.spec.ts index 7b829d21..ef696ebc 100644 --- a/packages/scan/src/models/Severity.spec.ts +++ b/packages/scan/src/models/Severity.spec.ts @@ -23,8 +23,8 @@ describe('Severity', () => { item.expected === 0 ? 'zero' : item.expected > 0 - ? 'positive' - : 'negative' + ? 'positive' + : 'negative' })) )( 'should return $expectedLabel comparing $input.a and $input.b', diff --git a/packages/scan/src/models/Tests.ts b/packages/scan/src/models/Tests.ts new file mode 100644 index 00000000..0919b071 --- /dev/null +++ b/packages/scan/src/models/Tests.ts @@ -0,0 +1,14 @@ +export type Test = string | BrokenAccessControlTest; + +export type BrokenAccessControlOptions = + | { + auth: string; // auth_object_id of the authorized user; scan compares access as this user vs as an unauthorized user + } + | { + auth: [string, string]; // auth_object_ids to scan with authorized users with different privileges + }; + +export type BrokenAccessControlTest = { + name: 'broken_access_control'; + options: BrokenAccessControlOptions; +}; diff --git a/packages/scan/src/models/index.ts b/packages/scan/src/models/index.ts index 1174366e..1892df1e 100644 --- a/packages/scan/src/models/index.ts +++ b/packages/scan/src/models/index.ts @@ -6,3 +6,4 @@ export * from './IssueGroup'; export * from './ScanState'; export * from './ScanConfig'; export * from './HttpMethod'; +export * from './Tests';