Skip to content

Commit

Permalink
[Backport 4.3-1.2-wzd] Sanitize report's inputs and usernames (#4332)
Browse files Browse the repository at this point in the history
Sanitize report's inputs and usernames (#4330)

* Validate path parameters of the reports endpoint

* Remove leftover prop in the reporting-table component

* Fix typo in report name validtion

* Remove the return of reports path from the /reports endpoint

* Remove absolute file path on report not found

(cherry picked from commit 3c1e51c)
  • Loading branch information
AlexRuiz7 authored Jul 15, 2022
1 parent 0e4b6bc commit 5ac79b6
Show file tree
Hide file tree
Showing 16 changed files with 951 additions and 92 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class WzReportingTable extends Component {
async getItems() {
try {
const rawItems = await this.reportingHandler.listReports();
const items = ((rawItems || {}).data || {}).reports || [];
const {reports: items = [], path} = rawItems?.data;
this.setState({
items,
isProcessing: false,
Expand Down
10 changes: 0 additions & 10 deletions public/react-services/reporting.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,16 +91,11 @@ export class ReportingService {
const appliedFilters = await this.visHandlers.getAppliedFilters(syscollectorFilters);

const array = await this.vis2png.checkArray(visualizationIDList);
const name = `wazuh-${
agents ? `agent-${agents}` : 'overview'
}-${tab}-${(Date.now() / 1000) | 0}.pdf`;

const browserTimezone = moment.tz.guess(true);

const data = {
array,
name,
title: agents ? `Agents ${tab}` : `Overview ${tab}`,
filters: appliedFilters.filters,
time: appliedFilters.time,
searchBar: appliedFilters.searchBar,
Expand Down Expand Up @@ -152,15 +147,10 @@ export class ReportingService {
this.$rootScope.reportStatus = 'Generating PDF document...';
this.$rootScope.$applyAsync();

const docType = type === 'agentConfig' ? `wazuh-agent-${obj.id}` : `wazuh-group-${obj.name}`;

const name = `${docType}-configuration-${(Date.now() / 1000) | 0}.pdf`;
const browserTimezone = moment.tz.guess(true);

const data = {
name,
filters: [type === 'agentConfig' ? { agent: obj.id } : { group: obj.name }],
tab: type,
browserTimezone,
components,
apiId: JSON.parse(AppState.getCurrentAPI()).id
Expand Down
220 changes: 220 additions & 0 deletions server/controllers/wazuh-reporting-security-endpoint-handler.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,220 @@
import md5 from 'md5';
import fs from 'fs';
import { WazuhReportingCtrl } from './wazuh-reporting';

jest.mock('../lib/logger', () => ({
log: jest.fn()
}));

jest.mock('../lib/reporting/printer', () => {
class ReportPrinterMock {
constructor() { }
addContent() { }
addConfigTables() { }
addTables() { }
addTimeRangeAndFilters() { }
addVisualizations() { }
formatDate() { }
checkTitle() { }
addSimpleTable() { }
addList() { }
addNewLine() { }
addContentWithNewLine() { }
addAgentsFilters() { }
print() { }
}
return {
ReportPrinter: ReportPrinterMock
}
});

const getMockerUserContext = (username: string) => ({ username, hashUsername: md5(username) });

const mockContext = (username: string) => ({
wazuh: {
security: {
getCurrentUser: () => getMockerUserContext(username)
}
}
});

const mockResponse = () => ({
ok: (body) => body,
custom: (body) => body,
badRequest: (body) => body
});

const endpointController = new WazuhReportingCtrl();

console.warn(`
Theses tests don't have in account the validation of endpoint parameters.
The endpoints related to an specific file.
This validation could prevent the endpoint handler is executed, so these tests
don't cover the reality.
`);

describe('[security] Report endpoints guard related to a file. Parameter defines or builds the filename.', () => {
let routeHandler = null;
const routeHandlerResponse = 'Endpoint handler executed.';

beforeEach(() => {
routeHandler = jest.fn(() => routeHandlerResponse);
});

afterEach(() => {
routeHandler = null;
});

it.each`
testTitle | username | filename | endpointProtected
${'Execute endpoint handler'} | ${'admin'} | ${'wazuh-module-overview-general-1234.pdf'} | ${false}
${'Endpoint protected'} | ${'admin'} | ${'../wazuh-module-overview-general-1234.pdf'} | ${true}
${'Endpoint protected'} | ${'admin'} | ${'wazuh-module-overview-../general-1234.pdf'} | ${true}
${'Endpoint protected'} | ${'admin'} | ${'custom../wazuh-module-overview-general-1234.pdf'} | ${true}
${'Execute endpoint handler'} | ${'../../etc'} | ${'wazuh-module-agents-001-general-1234.pdf'} | ${false}
${'Endpoint protected'} | ${'../../etc'} | ${'../wazuh-module-agents-001-general-1234.pdf'} | ${true}
${'Endpoint protected'} | ${'../../etc'} | ${'wazuh-module-overview-../general-1234.pdf'} | ${true}
${'Endpoint protected'} | ${'../../etc'} | ${'custom../wazuh-module-overview-general-1234.pdf'} | ${true}
`(`$testTitle
username: $username
filename: $filename
endpointProtected: $endpointProtected`, async ({ username, filename, endpointProtected }) => {
const response = await endpointController.checkReportsUserDirectoryIsValidRouteDecorator(
routeHandler,
function getFilename(request) {
return request.params.name
}
)(mockContext(username), { params: { name: filename } }, mockResponse());
if (endpointProtected) {

expect(response.body.message).toBe('5040 - You shall not pass!');
expect(routeHandler.mock.calls).toHaveLength(0);
} else {
expect(routeHandler.mock.calls).toHaveLength(1);
expect(response).toBe(routeHandlerResponse);
}
});

});

describe('[security] GET /reports', () => {

it.each`
username
${'admin'}
${'../../etc'}
`(`Get user reports: GET /reports
username: $username`, async ({ username }) => {
jest.spyOn(fs, 'readdirSync').mockImplementation(() => []);

const response = await endpointController.getReports(mockContext(username), {}, mockResponse());
expect(response.body.reports).toHaveLength(0);
});
});

describe('[security] GET /reports/{name}', () => {

it.each`
titleTest | username | filename | valid
${'Get report'} | ${'admin'} | ${'wazuh-module-overview-1234.pdf'} | ${true}
${'Endpoint protected'} | ${'admin'} | ${'../wazuh-module-overview-1234.pdf'} | ${false}
${'Get report'} | ${'../../etc'} | ${'wazuh-module-overview-1234.pdf'} | ${true}
${'Endpoint protected'} | ${'../../etc'} | ${'../wazuh-module-overview-1234.pdf'} | ${false}
`(`$titleTest: GET /reports/$filename
username: $username
valid: $valid`, async ({ username, filename, valid }) => {
const fileContent = 'content file';
jest.spyOn(fs, 'readFileSync').mockImplementation(() => fileContent);

const response = await endpointController.getReportByName(mockContext(username), { params: { name: filename } }, mockResponse());
if (valid) {
expect(response.headers['Content-Type']).toBe('application/pdf');
expect(response.body).toBe('content file');
} else {
expect(response.body.message).toBe('5040 - You shall not pass!');
}
});
});

describe('[security] POST /reports', () => {
jest.mock('../lib/filesystem', () => ({
createDataDirectoryIfNotExists: jest.fn()
}));

it.each`
titleTest | username | moduleID | valid
${'Get report'} | ${'admin'} | ${'general'} | ${true}
${'Endpoint protected'} | ${'admin'} | ${'../general'} | ${false}
${'Get report'} | ${'../../etc'} | ${'general'} | ${true}
${'Endpoint protected'} | ${'../../etc'} | ${'../general'} | ${false}
`(`$titleTest: POST /reports/modules/$moduleID
username: $username
valid: $valid`, async ({ username, moduleID, valid }) => {
jest.spyOn(endpointController, 'renderHeader').mockImplementation(() => true);
jest.spyOn(endpointController, 'sanitizeKibanaFilters').mockImplementation(() => [false, false]);
jest.spyOn(endpointController, 'extendedInformation').mockImplementation(() => true);

const mockRequest = {
body: {
array: [],
agents: false,
browserTimezone: '',
searchBar: '',
filters: [],
time: {
from: '',
to: ''
},
tables: [],
section: 'overview',
indexPatternTitle: 'wazuh-alerts-*',
apiId: 'default',
tab: moduleID
},
params: {
moduleID: moduleID
}
};

const response = await endpointController.createReportsModules(mockContext(username), mockRequest, mockResponse());

if (valid) {
expect(response.body.success).toBe(true);
expect(response.body.message).toMatch(new RegExp(`Report wazuh-module-overview-${moduleID}`));
} else {
expect(response.body.message).toBe('5040 - You shall not pass!');
};
});
});

describe('[security] DELETE /reports/<filename>', () => {
let mockFsUnlinkSync;

afterEach(() => {
mockFsUnlinkSync.mockClear();
});

it.each`
titleTest | username | filename | valid
${'Delete report'} | ${'admin'} | ${'wazuh-module-overview-1234.pdf'} | ${true}
${'Endpoint protected'} | ${'admin'} | ${'../wazuh-module-overview-1234.pdf'} | ${false}
${'Endpoint protected'} | ${'admin'} | ${'custom../wazuh-module-overview-1234.pdf'}| ${false}
${'Delete report'} | ${'../../etc'} | ${'wazuh-module-overview-1234.pdf'} | ${true}
${'Endpoint protected'} | ${'../../etc'} | ${'../wazuh-module-overview-1234.pdf'} | ${false}
${'Endpoint protected'} | ${'../../etc'} | ${'custom../wazuh-module-overview-1234.pdf'}| ${false}
`(`[security] DELETE /reports/$filename
username: $username
valid: $valid`, async ({ filename, username, valid }) => {
mockFsUnlinkSync = jest.spyOn(fs, 'unlinkSync').mockImplementation(() => { });

const response = await endpointController.deleteReportByName(mockContext(username), { params: { name: filename } }, mockResponse());

if (valid) {
expect(response.body.error).toBe(0);
expect(mockFsUnlinkSync.mock.calls).toHaveLength(1);
} else {
expect(response.body.message).toBe('5040 - You shall not pass!');
expect(mockFsUnlinkSync.mock.calls).toHaveLength(0);
};
});
});
Loading

0 comments on commit 5ac79b6

Please sign in to comment.