Skip to content

Commit a417ce2

Browse files
SNtGogSNtGog
committed
Enhance authData validation and processing to address security concerns. Introduce stricter provider name validation to prevent prototype pollution and NoSQL injection. Refactor diffAuthData and related logic for improved safety, clarity, and efficiency. Add extensive test coverage for edge cases and malicious input handling.
1 parent 387bb4e commit a417ce2

File tree

3 files changed

+332
-69
lines changed

3 files changed

+332
-69
lines changed

‎spec/Users.authdata.security.spec.js‎

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1856,6 +1856,211 @@ describe('AuthData Security Tests', () => {
18561856
expect(error.code).toBeDefined();
18571857
}
18581858
});
1859+
1860+
it('should reject provider names with prototype pollution attempts', async () => {
1861+
mockFetch(mockGpgamesLogin());
1862+
const user = await Parse.User.logInWith('gpgames', {
1863+
authData: { id: MOCK_USER_ID, code: 'C1' },
1864+
});
1865+
const sessionToken = user.getSessionToken();
1866+
1867+
// Try to use constructor as provider name (enumerable by default)
1868+
try {
1869+
await user.save(
1870+
{ authData: { constructor: { id: 'test' } } },
1871+
{ sessionToken }
1872+
);
1873+
// If it doesn't throw, verify that constructor was not stored as malicious data
1874+
await user.fetch({ sessionToken });
1875+
const authData = user.get('authData');
1876+
// Constructor should not be in authData with our test data
1877+
// (either rejected or not configured, or stored but not as our test object)
1878+
if (authData && authData.constructor) {
1879+
const isOurTestData = typeof authData.constructor === 'object' &&
1880+
authData.constructor.id === 'test';
1881+
expect(isOurTestData).toBe(false);
1882+
}
1883+
} catch (error) {
1884+
// Should reject with INVALID_KEY_NAME (either from validateAuthData or diffAuthData)
1885+
// If provider is not configured, may throw UNSUPPORTED_SERVICE, which is also acceptable
1886+
expect([Parse.Error.INVALID_KEY_NAME, Parse.Error.UNSUPPORTED_SERVICE]).toContain(error.code);
1887+
if (error.code === Parse.Error.INVALID_KEY_NAME) {
1888+
expect(error.message).toContain('Invalid provider name');
1889+
}
1890+
}
1891+
1892+
// Try to use prototype as provider name
1893+
try {
1894+
await user.save(
1895+
{ authData: { prototype: { id: 'test' } } },
1896+
{ sessionToken }
1897+
);
1898+
// If it doesn't throw, verify that prototype was not stored as malicious data
1899+
await user.fetch({ sessionToken });
1900+
const authData = user.get('authData');
1901+
// Prototype should not be in authData with our test data
1902+
// (either rejected or not configured, or stored but not as our test object)
1903+
if (authData && authData.prototype) {
1904+
const isOurTestData = typeof authData.prototype === 'object' &&
1905+
authData.prototype.id === 'test';
1906+
expect(isOurTestData).toBe(false);
1907+
}
1908+
} catch (error) {
1909+
// Should reject with INVALID_KEY_NAME (either from validateAuthData or diffAuthData)
1910+
// If provider is not configured, may throw UNSUPPORTED_SERVICE, which is also acceptable
1911+
expect([Parse.Error.INVALID_KEY_NAME, Parse.Error.UNSUPPORTED_SERVICE]).toContain(error.code);
1912+
if (error.code === Parse.Error.INVALID_KEY_NAME) {
1913+
expect(error.message).toContain('Invalid provider name');
1914+
}
1915+
}
1916+
1917+
// Note: __proto__ is not enumerable by default in Object.keys(),
1918+
// so it won't be processed by diffAuthData. However, if it were
1919+
// enumerable (via Object.defineProperty with enumerable: true),
1920+
// it would be rejected by our validation. This is acceptable because
1921+
// normal object literals don't have enumerable __proto__.
1922+
});
1923+
1924+
it('should reject provider names with NoSQL injection attempts', async () => {
1925+
mockFetch(mockGpgamesLogin());
1926+
const user = await Parse.User.logInWith('gpgames', {
1927+
authData: { id: MOCK_USER_ID, code: 'C1' },
1928+
});
1929+
const sessionToken = user.getSessionToken();
1930+
1931+
// Try to use dots in provider name (NoSQL injection)
1932+
const maliciousProviders = [
1933+
'gpgames.id',
1934+
'gpgames.access_token',
1935+
'provider.name.field',
1936+
'test.test',
1937+
];
1938+
1939+
for (const maliciousProvider of maliciousProviders) {
1940+
try {
1941+
await user.save(
1942+
{ authData: { [maliciousProvider]: { id: 'test' } } },
1943+
{ sessionToken }
1944+
);
1945+
fail(`Should have rejected provider name with dots: ${maliciousProvider}`);
1946+
} catch (error) {
1947+
expect(error.code).toBe(Parse.Error.INVALID_KEY_NAME);
1948+
expect(error.message).toContain('Invalid provider name');
1949+
}
1950+
}
1951+
});
1952+
1953+
it('should reject provider names with special characters', async () => {
1954+
mockFetch(mockGpgamesLogin());
1955+
const user = await Parse.User.logInWith('gpgames', {
1956+
authData: { id: MOCK_USER_ID, code: 'C1' },
1957+
});
1958+
const sessionToken = user.getSessionToken();
1959+
1960+
// Try various special characters
1961+
const maliciousProviders = [
1962+
'provider-name', // hyphen
1963+
'provider name', // space
1964+
'provider@name', // @
1965+
'provider#name', // #
1966+
'provider$name', // $
1967+
'provider[name]', // brackets
1968+
'provider{name}', // braces
1969+
'provider/name', // slash
1970+
'provider\\name', // backslash
1971+
'provider.name', // dot
1972+
'123provider', // starts with number
1973+
'', // empty string
1974+
];
1975+
1976+
for (const maliciousProvider of maliciousProviders) {
1977+
try {
1978+
await user.save(
1979+
{ authData: { [maliciousProvider]: { id: 'test' } } },
1980+
{ sessionToken }
1981+
);
1982+
fail(`Should have rejected invalid provider name: ${maliciousProvider}`);
1983+
} catch (error) {
1984+
expect(error.code).toBe(Parse.Error.INVALID_KEY_NAME);
1985+
expect(error.message).toContain('Invalid provider name');
1986+
}
1987+
}
1988+
});
1989+
1990+
it('should accept valid provider names', async () => {
1991+
mockFetch(mockGpgamesLogin());
1992+
const user = await Parse.User.logInWith('gpgames', {
1993+
authData: { id: MOCK_USER_ID, code: 'C1' },
1994+
});
1995+
const sessionToken = user.getSessionToken();
1996+
1997+
// Valid provider names should work
1998+
const validProviders = [
1999+
'gpgames',
2000+
'instagram',
2001+
'provider_name',
2002+
'providerName',
2003+
'ProviderName',
2004+
'provider123',
2005+
'p',
2006+
'provider_name_123',
2007+
];
2008+
2009+
for (const validProvider of validProviders) {
2010+
// Skip if provider is not configured (will fail with different error)
2011+
if (validProvider === 'gpgames' || validProvider === 'instagram') {
2012+
continue;
2013+
}
2014+
2015+
try {
2016+
await user.save(
2017+
{ authData: { [validProvider]: { id: 'test' } } },
2018+
{ sessionToken }
2019+
);
2020+
// Should not throw INVALID_KEY_NAME error
2021+
// May throw UNSUPPORTED_SERVICE if provider not configured, which is expected
2022+
} catch (error) {
2023+
// Should not be INVALID_KEY_NAME for valid provider names
2024+
expect(error.code).not.toBe(Parse.Error.INVALID_KEY_NAME);
2025+
}
2026+
}
2027+
});
2028+
2029+
it('should prevent injection when linking multiple providers', async () => {
2030+
mockFetch(mockGpgamesLogin());
2031+
const user = await Parse.User.logInWith('gpgames', {
2032+
authData: { id: MOCK_USER_ID, code: 'C1' },
2033+
});
2034+
const sessionToken = user.getSessionToken();
2035+
2036+
// Try to link malicious provider name
2037+
// Use constructor which should be enumerable
2038+
try {
2039+
await user.save(
2040+
{
2041+
authData: {
2042+
constructor: { id: 'test' },
2043+
},
2044+
},
2045+
{ sessionToken }
2046+
);
2047+
// If it doesn't throw, verify that constructor was not stored
2048+
await user.fetch({ sessionToken });
2049+
const authData = user.get('authData');
2050+
// Constructor should not be in authData (either rejected or not configured)
2051+
const hasConstructor = authData && authData.constructor &&
2052+
typeof authData.constructor === 'object' &&
2053+
authData.constructor.id === 'test';
2054+
expect(hasConstructor).toBe(false);
2055+
} catch (error) {
2056+
// Should reject with INVALID_KEY_NAME (either from validateAuthData or diffAuthData)
2057+
// If provider is not configured, may throw UNSUPPORTED_SERVICE, which is also acceptable
2058+
expect([Parse.Error.INVALID_KEY_NAME, Parse.Error.UNSUPPORTED_SERVICE]).toContain(error.code);
2059+
if (error.code === Parse.Error.INVALID_KEY_NAME) {
2060+
expect(error.message).toContain('Invalid provider name');
2061+
}
2062+
}
2063+
});
18592064
});
18602065
});
18612066

‎src/Auth.js‎

Lines changed: 80 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -645,50 +645,93 @@ const handleAuthDataValidation = async (authData, req, foundUser) => {
645645
return acc;
646646
};
647647

648-
const subsetEqual = (prev, next) => {
649-
if (prev === next) return true;
650-
if (prev == null || next == null) return false;
651-
652-
const tp = typeof prev;
653-
const tn = typeof next;
654-
if (tn !== 'object' || tp !== 'object') return prev === next;
655-
656-
if (Array.isArray(prev) && Array.isArray(next)) {
657-
if (next.length > prev.length) return false;
658-
for (let i = 0; i < next.length; i++) {
659-
if (!subsetEqual(prev[i], next[i])) {
660-
return false;
661-
}
662-
}
663-
return true;
648+
const hasOwn = (o, k) => Object.prototype.hasOwnProperty.call(o, k);
649+
650+
const toRecord = (v) => {
651+
const out = Object.create(null);
652+
if (!v || typeof v !== 'object' || Array.isArray(v)) return out;
653+
for (const k of Object.keys(v)) {
654+
out[k] = v[k];
664655
}
656+
return out;
657+
};
665658

666-
if (Array.isArray(prev) !== Array.isArray(next)) {
667-
return false;
659+
const assertSafeProviderKey = (p) => {
660+
if (p === '__proto__' || p === 'constructor' || p === 'prototype') {
661+
throw new Parse.Error(
662+
Parse.Error.INVALID_KEY_NAME,
663+
`Invalid provider name: ${p}. Provider names cannot be reserved JavaScript properties.`
664+
);
665+
}
666+
if (!/^[A-Za-z][A-Za-z0-9_]*$/.test(p)) {
667+
throw new Parse.Error(
668+
Parse.Error.INVALID_KEY_NAME,
669+
`Invalid provider name: ${p}. Provider names must start with a letter and contain only alphanumeric characters and underscores.`
670+
);
668671
}
672+
};
669673

670-
for (const key in next) {
671-
if (!(key in prev)) {
672-
return false;
673-
}
674-
if (!subsetEqual(prev[key], next[key])) {
675-
return false;
676-
}
674+
const assertProviderData = (x) => {
675+
if (x === null || typeof x === 'undefined') return;
676+
if (!x || typeof x !== 'object' || Array.isArray(x)) {
677+
throw new Parse.Error(
678+
Parse.Error.VALIDATION_ERROR,
679+
'Invalid provider data.'
680+
);
681+
}
682+
if (Object.keys(x).length > 32) {
683+
throw new Parse.Error(
684+
Parse.Error.VALIDATION_ERROR,
685+
'Provider data too large.'
686+
);
687+
}
688+
if (typeof x.id !== 'undefined' && (typeof x.id !== 'string' || x.id.length > 256)) {
689+
throw new Parse.Error(
690+
Parse.Error.VALIDATION_ERROR,
691+
'Invalid provider id.'
692+
);
677693
}
694+
};
678695

696+
const shallowStableEqual = (a, b) => {
697+
if (a === b) return true;
698+
if (!a || !b || typeof a !== 'object' || typeof b !== 'object' || Array.isArray(a) || Array.isArray(b)) {
699+
return false;
700+
}
701+
const ak = Object.keys(a);
702+
const bk = Object.keys(b);
703+
if (ak.length !== bk.length) return false;
704+
for (const k of ak) {
705+
if (!hasOwn(b, k)) return false;
706+
if (a[k] !== b[k]) return false;
707+
}
679708
return true;
680-
}
709+
};
681710

682711
const diffAuthData = (current = {}, incoming = {}) => {
683-
const changed = {};
684-
const unlink = {};
685-
const unchanged = {};
712+
// Convert to safe records (null-prototype) for internal processing
713+
const cur = toRecord(current);
714+
const inc = toRecord(incoming);
715+
716+
// Use null-prototype objects internally to prevent prototype pollution
717+
const changed = Object.create(null);
718+
const unlink = Object.create(null);
719+
const unchanged = Object.create(null);
686720

687-
const providers = _.union(Object.keys(current), Object.keys(incoming));
721+
const providers = _.union(Object.keys(cur), Object.keys(inc));
722+
723+
if (providers.length > 32) {
724+
throw new Parse.Error(
725+
Parse.Error.VALIDATION_ERROR,
726+
'Too many providers. Maximum 32 providers allowed.'
727+
);
728+
}
688729

689730
for (const p of providers) {
690-
const prev = current[p];
691-
const next = incoming[p];
731+
assertSafeProviderKey(p);
732+
733+
const prev = hasOwn(cur, p) ? cur[p] : undefined;
734+
const next = hasOwn(inc, p) ? inc[p] : undefined;
692735

693736
if (next === null) {
694737
unlink[p] = true;
@@ -700,6 +743,8 @@ const diffAuthData = (current = {}, incoming = {}) => {
700743
continue;
701744
}
702745

746+
assertProviderData(next);
747+
703748
if (_.isUndefined(prev)) {
704749
changed[p] = next;
705750
continue;
@@ -712,14 +757,14 @@ const diffAuthData = (current = {}, incoming = {}) => {
712757
continue;
713758
}
714759

715-
if (isDeepStrictEqual(prev, next)) {
716-
unchanged[p] = prev;
717-
} else if (subsetEqual(prev, next)) {
760+
if (shallowStableEqual(prev, next) || isDeepStrictEqual(prev, next)) {
718761
unchanged[p] = prev;
719762
} else {
720763
changed[p] = next;
721764
}
722765
}
766+
// Return null-prototype objects to maintain protection against prototype pollution
767+
// Object.keys() works perfectly with null-prototype objects
723768
return { changed, unlink, unchanged };
724769
};
725770

@@ -736,6 +781,5 @@ module.exports = {
736781
hasMutatedAuthData,
737782
checkIfUserHasProvidedConfiguredProvidersForLogin,
738783
handleAuthDataValidation,
739-
subsetEqual,
740784
diffAuthData
741785
};

0 commit comments

Comments
 (0)