Skip to content

Commit

Permalink
ADOP-2523 Escaping characters to prevent HTML injection (#1587)
Browse files Browse the repository at this point in the history
* [ADOP-2523] removed redundant code

* [ADOP-2523] Removed duplicated code tech debt
  • Loading branch information
hfilmore authored Oct 10, 2024
1 parent bd9d160 commit ba874b7
Show file tree
Hide file tree
Showing 9 changed files with 156 additions and 1,299 deletions.
1 change: 1 addition & 0 deletions src/main/app/case/case.ts
Original file line number Diff line number Diff line change
Expand Up @@ -500,4 +500,5 @@ export enum FieldPrefix {
BIRTH_FATHER = 'birthFather',
BIRTH_MOTHER = 'birthMother',
OTHER_PARENT = 'otherParent',
OTHER_ADOPTION_AGENCY = 'adopAgency',
}
29 changes: 29 additions & 0 deletions src/main/app/case/formatter/address.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ describe('address', () => {
applicant1AddressTown: 'MOCK_ADDRESS_TOWN',
applicant1AddressCounty: 'MOCK_ADDRESS_COUNTY',
applicant1AddressPostcode: 'MOCK_ADDRESS_POSTCODE',
adopAgencyAddressLine1: 'ADOP_MOCK_ADDRESS_1',
adopAgencyTown: 'ADOP_MOCK_ADDRESS_TOWN',
adopAgencyPostcode: 'ADOP_MOCK_ADDRESS_POSTCODE',
},
expected:
'MOCK_ADDRESS_1<br>MOCK_ADDRESS_2<br>MOCK_ADDRESS_TOWN<br>MOCK_ADDRESS_COUNTY<br>MOCK_ADDRESS_POSTCODE',
Expand All @@ -25,3 +28,29 @@ describe('address', () => {
});
});
});

describe('adopAgency address', () => {
describe('getFormattedAddress', () => {
test.each([
{
data: {},
expected: '',
},
{
data: {
applicant1Address1: 'MOCK_ADDRESS_1',
applicant1Address2: 'MOCK_ADDRESS_2',
applicant1AddressTown: 'MOCK_ADDRESS_TOWN',
applicant1AddressCounty: 'MOCK_ADDRESS_COUNTY',
applicant1AddressPostcode: 'MOCK_ADDRESS_POSTCODE',
adopAgencyAddressLine1: 'ADOP_MOCK_ADDRESS_1',
adopAgencyTown: 'ADOP_MOCK_ADDRESS_TOWN',
adopAgencyPostcode: 'ADOP_MOCK_ADDRESS_POSTCODE',
},
expected: 'ADOP_MOCK_ADDRESS_1<br>ADOP_MOCK_ADDRESS_TOWN<br>ADOP_MOCK_ADDRESS_POSTCODE',
},
])('should return correct formatted other adoption agency address', ({ data, expected }) => {
expect(getFormattedAddress(data, FieldPrefix.OTHER_ADOPTION_AGENCY)).toEqual(expected);
});
});
});
23 changes: 16 additions & 7 deletions src/main/app/case/formatter/address.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,27 @@
import { sanitizeHtmlArray } from '../../../steps/common/functions/sanitize';
import { Case, FieldPrefix } from '../case';

export const getFormattedAddress = (data: Partial<Case>, prefix: FieldPrefix): string => {
let address: string[] = [];

address.push(data[`${prefix}Address1`] || '');
address.push(data[`${prefix}Address2`] || '');
address.push(data[`${prefix}Address3`] || '');
address.push(data[`${prefix}AddressTown`] || '');
address.push(data[`${prefix}AddressCounty`] || '');
address.push(data[`${prefix}AddressPostcode`] || '');
address.push(data[`${prefix}AddressCountry`] || '');
if (prefix === FieldPrefix.OTHER_ADOPTION_AGENCY) {
address.push(data[`${prefix}AddressLine1`] || '');
address.push(data[`${prefix}Town`] || '');
address.push(data[`${prefix}Postcode`] || '');
} else {
address.push(data[`${prefix}Address1`] || '');
address.push(data[`${prefix}Address2`] || '');
address.push(data[`${prefix}Address3`] || '');
address.push(data[`${prefix}AddressTown`] || '');
address.push(data[`${prefix}AddressCounty`] || '');
address.push(data[`${prefix}AddressPostcode`] || '');
address.push(data[`${prefix}AddressCountry`] || '');
}

//remove empty items
address = address.filter(item => !!item);

address = sanitizeHtmlArray(address);

return address.join('<br>');
};
12 changes: 10 additions & 2 deletions src/main/steps/applicant2/same-address/content.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { FieldPrefix } from '../../../app/case/case';
import { YesOrNo } from '../../../app/case/definition';
import { getFormattedAddress } from '../../../app/case/formatter/address';
import { TranslationFn } from '../../../app/controller/GetController';
import { FormContent } from '../../../app/form/Form';
import { isFieldFilledIn } from '../../../app/form/validation';
Expand All @@ -8,7 +10,10 @@ const en = content => ({
title: 'Do you also live at this address?',
yes: 'Yes',
no: 'No',
applicant1Address: `<div class="govuk-inset-text">${content.userCase.applicant1Address1}<br>${content.userCase.applicant1AddressTown}<br>${content.userCase.applicant1AddressPostcode}</div>`,
applicant1Address: `<div class="govuk-inset-text">${getFormattedAddress(
content.userCase,
FieldPrefix.APPLICANT1
)}</div>`,
errors: {
applicant2AddressSameAsApplicant1: {
required: 'Please answer the question',
Expand All @@ -21,7 +26,10 @@ const cy: typeof en = content => ({
title: 'A ydych chi’n byw yn y cyfeiriad hwn hefyd?',
yes: 'Ydw',
no: 'Nac ydw',
applicant1Address: `<div class="govuk-inset-text">${content.userCase.applicant1Address1}<br>${content.userCase.applicant1AddressTown}<br>${content.userCase.applicant1AddressPostcode}</div>`,
applicant1Address: `<div class="govuk-inset-text">${getFormattedAddress(
content.userCase,
FieldPrefix.APPLICANT1
)}</div>`,
errors: {
applicant2AddressSameAsApplicant1: {
required: 'Atebwch y cwestiwn os gwelwch yn dda',
Expand Down
37 changes: 37 additions & 0 deletions src/main/steps/common/functions/sanitize.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { sanitizeHtml, sanitizeHtmlArray } from './sanitize';

test('Changes nothing in a string with no special characters', () => {
const testString = 'This is a test string 123';
expect(sanitizeHtml(testString)).toBe(testString);
});

test('Returns undefined where input is undefined', () => {
let testString;
expect(sanitizeHtml(testString)).toBe(undefined);
});

test('Escapes special characters when input contains HTML', () => {
const input = '<h2>HTML Injection</h2>';
const expected = '&lt;h2&gt;HTML Injection&lt;&#x2F;h2&gt;';
expect(sanitizeHtml(input)).toBe(expected);
});

test('Returns empty array when passed empty array', () => {
const testArray = [];
expect(sanitizeHtmlArray(testArray)).toStrictEqual([]);
});

test('Return undefined where array is undefined', () => {
let testArray;
expect(sanitizeHtmlArray(testArray)).toStrictEqual(undefined);
});

test('Escapes special characters when input array contains HTML', () => {
const testArray = ['<h2>HTML Injection</h2>', "321 St Christopher-Walden's", 'Nothing to see here'];
const expected = [
'&lt;h2&gt;HTML Injection&lt;&#x2F;h2&gt;',
'321 St Christopher-Walden&#x27;s',
'Nothing to see here',
];
expect(sanitizeHtmlArray(testArray)).toStrictEqual(expected);
});
25 changes: 25 additions & 0 deletions src/main/steps/common/functions/sanitize.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
export function sanitizeHtml(input: string): string {
const map: Record<string, string> = {
'&': '&amp;',
'<': '&lt;',
'>': '&gt;',
'"': '&quot;',
"'": '&#x27;',
'/': '&#x2F;',
'`': '&#x60;',
'=': '&#x3D;',
};
const reg = /[&<>"'`=/]/gi;

if (input) {
return input.replace(reg, match => map[match]);
}
return input;
}

export function sanitizeHtmlArray(input: string[]): string[] {
if (input) {
return input.map(item => sanitizeHtml(item));
}
return input;
}
16 changes: 9 additions & 7 deletions src/main/steps/la-portal/check-your-answers/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
} from '../../../app/case/definition';
import { getFormattedAddress } from '../../../app/case/formatter/address';
import { PageContent } from '../../../app/controller/GetController';
import { sanitizeHtml, sanitizeHtmlArray } from '../../../steps/common/functions/sanitize';
import * as Urls from '../../../steps/urls';

interface GovUKNunjucksSummary {
Expand Down Expand Up @@ -160,7 +161,7 @@ const formatNationalities = (
if (nationality.includes(Nationality.OTHER)) {
nationalities.push(...additionalNationalities.map(item => item.country));
}
return nationalities.join('<br>');
return sanitizeHtmlArray(nationalities).join('<br>');
};

const formatResponsibilityReasons = (
Expand All @@ -169,15 +170,16 @@ const formatResponsibilityReasons = (
reasonsProvided,
reasonText: string
): string => {
const safeOtherReasonText = sanitizeHtml(otherReasonText);
const responsibilities = responsibility.filter(item => item !== ResponsibilityReasons.OTHER);
const translatedResponsibilities: string[] = [];
const responsibilityText = `<p class="govuk-!-margin-top-0"><span class="govuk-!-font-weight-bold">${reasonText} </span> `;
responsibilities.forEach(element => translatedResponsibilities.push(reasonsProvided[element]));

if (responsibility.includes(ResponsibilityReasons.OTHER) && translatedResponsibilities.length === 0) {
return responsibilityText + otherReasonText;
return responsibilityText + safeOtherReasonText;
} else if (responsibility.includes(ResponsibilityReasons.OTHER)) {
return responsibilityText + translatedResponsibilities.join('<br>') + '<br>' + otherReasonText;
return responsibilityText + translatedResponsibilities.join('<br>') + '<br>' + safeOtherReasonText;
}
return responsibilityText + translatedResponsibilities.join('<br>');
};
Expand Down Expand Up @@ -402,7 +404,9 @@ function lastAddressKnown(
}

const getNotSureReasonElement = (content, userCase, notSure, reasonFieldName): string => {
return `${notSure}<p class="govuk-!-margin-top-0"><span class="govuk-!-font-weight-bold">${content.reason}: </span>${userCase[reasonFieldName]}</p>`;
return `${notSure}<p class="govuk-!-margin-top-0"><span class="govuk-!-font-weight-bold">${
content.reason
}: </span>${sanitizeHtml(userCase[reasonFieldName])}</p>`;
};

export const otherParentSummaryList = (
Expand Down Expand Up @@ -431,9 +435,7 @@ export const otherParentSummaryList = (
visuallyHiddenText: visuallyHiddenTexts.otherParentResponsibilityReason,
valueHtml: formatResponsibilityReasons(
userCase.otherParentResponsibilityReason!,
userCase.otherParentOtherResponsibilityReason
? content.responsibilityReasons['Other'] + '<br>' + userCase.otherParentOtherResponsibilityReason
: '',
userCase.otherParentOtherResponsibilityReason ? userCase.otherParentOtherResponsibilityReason : '',
content.responsibilityReasons,
''
),
Expand Down
Loading

0 comments on commit ba874b7

Please sign in to comment.