Skip to content

Commit

Permalink
Merge pull request from GHSA-7ww6-75fj-jcj7
Browse files Browse the repository at this point in the history
* Sanitize additional signup fields using DOMPurify

* Remove HTML entirely from additionalSignupFields
  • Loading branch information
Steve Hobbs authored May 5, 2022
1 parent 7280665 commit 79ae557
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 2 deletions.
71 changes: 71 additions & 0 deletions src/__tests__/connection/database/actions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,4 +137,75 @@ describe('database/actions.js', () => {
expect(signUpMock.calls.length).toBe(1);
expect(signUpMock.calls[0][0]).toBe(id);
});

it('sanitizes additionalSignUp fields using dompurify', () => {
const id = 1;
const hookRunner = jest.fn((str, m, context, fn) => fn());

require('connection/database/index').databaseConnectionName = () => 'test-connection';
require('connection/database/index').shouldAutoLogin = () => true;

// Test different fields using some examples from DOMPurify
// https://github.com/cure53/DOMPurify#some-purification-samples-please
const m = Immutable.fromJS({
field: {
email: {
value: '[email protected]'
},
password: {
value: 'testpass'
},
family_name: {
value: 'Test <a href="https://www.google.co.uk">Fake link</a>' // HTML but not malicious
},
given_name: {
value: '<img src=x onerror=alert(1)//>'
},
name: {
value: '<p>abc<iframe//src=jAva&Tab;script:alert(3)>def</p>'
},
other_name: {
value:
'<div onclick=alert(0)><form onsubmit=alert(1)><input onfocus=alert(2) name=parentNode>123</form></div>'
}
},
database: {
additionalSignUpFields: [
{ name: 'family_name', storage: 'root' },
{ name: 'given_name', storage: 'root' },
{ name: 'name', storage: 'root' },
{ name: 'other_name' }
]
},
core: {
hookRunner
}
});

swap(setEntity, 'lock', id, m);
signUp(id);

const {
validateAndSubmit: { mock: validateAndSubmitMock }
} = coreActionsMock();

validateAndSubmitMock.calls[0][2](m);

const {
signUp: { mock: signUpMock }
} = webApiMock();

expect(signUpMock.calls[0][1]).toMatchObject({
connection: 'test-connection',
email: '[email protected]',
password: 'testpass',
autoLogin: true,
family_name: 'Test Fake link',
given_name: '',
name: 'abc',
user_metadata: {
other_name: '123'
}
});
});
});
6 changes: 4 additions & 2 deletions src/connection/database/actions.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import Immutable, { Map } from 'immutable';
import { getEntity, read, swap, updateEntity } from '../../store/index';
import webApi from '../../core/web_api';
import { closeLock, logIn as coreLogIn, logInSuccess, validateAndSubmit } from '../../core/actions';
import * as l from '../../core/index';
import * as c from '../../field/index';
import { sanitize } from 'dompurify';

import {
databaseConnection,
databaseConnectionName,
Expand Down Expand Up @@ -107,7 +108,8 @@ export function signUp(id) {
additionalSignUpFields(m).forEach(x => {
const storage = x.get('storage');
const fieldName = x.get('name');
const fieldValue = c.getFieldValue(m, x.get('name'));
const fieldValue = sanitize(c.getFieldValue(m, x.get('name')), { ALLOWED_TAGS: [] });

switch (storage) {
case 'root':
params[fieldName] = fieldValue;
Expand Down

0 comments on commit 79ae557

Please sign in to comment.