Skip to content

Commit a2230e9

Browse files
authored
PLU-485: add sanitisation for null characters for formsg submission (#1048)
## Problem Null characters are not sanitised when the form submission is made and an error is thrown when inserting into our `ExecutionSteps` table DB ## Solution Replace `\u0000` characters with empty spaces before inserting the data into our DB ## Tests - [ ] Check that `\u0000` characters are being sanitised for answer fields e.g. short answer, long answer - [ ] Check that `\u0000` characters are being sanitised for 1D answerArray fields e.g. checkbox, address - [ ] Check that `\u0000` characters are being sanitised for 2D answerArray fields e.g. table
1 parent d2826b6 commit a2230e9

File tree

2 files changed

+202
-0
lines changed

2 files changed

+202
-0
lines changed

packages/backend/src/apps/formsg/__tests__/auth/decrypt-form-response.test.ts

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -636,4 +636,186 @@ describe('decrypt form response', () => {
636636
)
637637
})
638638
})
639+
640+
describe('null character sanitisation', () => {
641+
it('should handle normal answer field with null characters', async () => {
642+
$.flow.hasFileProcessingActions = false
643+
mocks.cryptoDecrypt.mockReturnValueOnce({
644+
responses: [
645+
{
646+
_id: 'textField',
647+
fieldType: 'text',
648+
question: 'What is your name?',
649+
answer: 'John\u0000 Tan\u0000',
650+
},
651+
],
652+
})
653+
654+
await expect(decryptFormResponse($)).resolves.toEqual(
655+
SUCCESS_DECRYPT_RESPONSE,
656+
)
657+
expect($.request.body).toEqual(
658+
expect.objectContaining({
659+
fields: {
660+
textField: {
661+
fieldType: 'text',
662+
question: 'What is your name?',
663+
answer: 'John Tan',
664+
order: 1,
665+
},
666+
},
667+
}),
668+
)
669+
})
670+
671+
it('should handle 1D array field answerArray', async () => {
672+
$.flow.hasFileProcessingActions = false
673+
mocks.cryptoDecrypt.mockReturnValueOnce({
674+
responses: [
675+
{
676+
_id: 'checkboxField',
677+
fieldType: 'checkbox',
678+
question: 'What are your hobbies?',
679+
answerArray: ['reading', 'gaming', 'coding'],
680+
},
681+
],
682+
})
683+
684+
await expect(decryptFormResponse($)).resolves.toEqual(
685+
SUCCESS_DECRYPT_RESPONSE,
686+
)
687+
expect($.request.body).toEqual(
688+
expect.objectContaining({
689+
fields: {
690+
checkboxField: {
691+
fieldType: 'checkbox',
692+
question: 'What are your hobbies?',
693+
answerArray: ['reading', 'gaming', 'coding'],
694+
order: 1,
695+
},
696+
},
697+
}),
698+
)
699+
})
700+
701+
it('should handle 2D array field answerArray', async () => {
702+
$.flow.hasFileProcessingActions = false
703+
mocks.cryptoDecrypt.mockReturnValueOnce({
704+
responses: [
705+
{
706+
_id: 'tableField',
707+
fieldType: 'table',
708+
question: 'What are your hobbies and when do you do them?',
709+
answerArray: [
710+
['reading', 'night'],
711+
['gaming', 'weekend'],
712+
['coding', 'day'],
713+
],
714+
},
715+
],
716+
})
717+
718+
await expect(decryptFormResponse($)).resolves.toEqual(
719+
SUCCESS_DECRYPT_RESPONSE,
720+
)
721+
expect($.request.body).toEqual(
722+
expect.objectContaining({
723+
fields: {
724+
tableField: {
725+
fieldType: 'table',
726+
question: 'What are your hobbies and when do you do them?',
727+
answerArray: [
728+
['reading', 'night'],
729+
['gaming', 'weekend'],
730+
['coding', 'day'],
731+
],
732+
order: 1,
733+
},
734+
},
735+
}),
736+
)
737+
})
738+
739+
it('should handle answerArray with null characters', async () => {
740+
$.flow.hasFileProcessingActions = false
741+
mocks.cryptoDecrypt.mockReturnValueOnce({
742+
responses: [
743+
{
744+
_id: 'checkboxField',
745+
fieldType: 'checkbox',
746+
question: 'What are your hobbies?',
747+
answerArray: [
748+
'reading\u0000',
749+
'\u0000gaming\u0000',
750+
'\u0000coding',
751+
],
752+
},
753+
{
754+
_id: 'tableField',
755+
fieldType: 'table',
756+
question: 'What are your hobbies and when do you do them?',
757+
answerArray: [
758+
['reading\u0000', 'night\u0000'],
759+
['gaming\u0000', 'weekend\u0000\u0000\u0000'],
760+
],
761+
},
762+
],
763+
})
764+
765+
await expect(decryptFormResponse($)).resolves.toEqual(
766+
SUCCESS_DECRYPT_RESPONSE,
767+
)
768+
expect($.request.body).toEqual(
769+
expect.objectContaining({
770+
fields: {
771+
checkboxField: {
772+
fieldType: 'checkbox',
773+
question: 'What are your hobbies?',
774+
answerArray: ['reading', 'gaming', 'coding'],
775+
order: 1,
776+
},
777+
tableField: {
778+
fieldType: 'table',
779+
question: 'What are your hobbies and when do you do them?',
780+
answerArray: [
781+
['reading', 'night'],
782+
['gaming', 'weekend'],
783+
],
784+
order: 2,
785+
},
786+
},
787+
}),
788+
)
789+
})
790+
791+
it('should handle empty answerArray', async () => {
792+
$.flow.hasFileProcessingActions = false
793+
mocks.cryptoDecrypt.mockReturnValueOnce({
794+
responses: [
795+
{
796+
_id: 'checkboxField',
797+
fieldType: 'checkbox',
798+
question: 'What are your hobbies?',
799+
answerArray: [],
800+
},
801+
],
802+
})
803+
804+
await expect(decryptFormResponse($)).resolves.toEqual(
805+
SUCCESS_DECRYPT_RESPONSE,
806+
)
807+
expect($.request.body).toEqual(
808+
expect.objectContaining({
809+
fields: {
810+
checkboxField: {
811+
fieldType: 'checkbox',
812+
question: 'What are your hobbies?',
813+
answerArray: [],
814+
order: 1,
815+
},
816+
},
817+
}),
818+
)
819+
})
820+
})
639821
})

packages/backend/src/apps/formsg/auth/decrypt-form-response.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,26 @@ export async function decryptFormResponse(
114114

115115
for (const [index, formField] of submission.responses.entries()) {
116116
const { _id, ...rest } = formField
117+
// perform null character sanitisation for all answer fields so that it can be inserted into the DB
118+
if (rest.answer) {
119+
rest.answer = rest.answer.replaceAll('\u0000', '')
120+
}
121+
122+
if (rest.answerArray && rest.answerArray.length > 0) {
123+
// Could be an array of strings (checkbox/address field) or a 2-D array of strings (table field)
124+
if (
125+
formField.fieldType === 'table' &&
126+
Array.isArray(rest.answerArray[0])
127+
) {
128+
rest.answerArray = (rest.answerArray as string[][]).map((row) =>
129+
row.map((column) => column.replaceAll('\u0000', '')),
130+
)
131+
} else {
132+
rest.answerArray = (rest.answerArray as string[]).map((answer) =>
133+
answer.replaceAll('\u0000', ''),
134+
)
135+
}
136+
}
117137

118138
if (rest.fieldType === 'nric' && !!rest.answer) {
119139
const filteredAnswer = filterNric($, rest.answer)

0 commit comments

Comments
 (0)