diff --git a/spec/ParseGraphQLServer.spec.js b/spec/ParseGraphQLServer.spec.js index eeb1ff7333..9fa49ce1f1 100644 --- a/spec/ParseGraphQLServer.spec.js +++ b/spec/ParseGraphQLServer.spec.js @@ -1427,6 +1427,340 @@ describe('ParseGraphQLServer', () => { expect(error.message).toContain('DiagBookWhereInput'); } }); + + // graphql-js also names a generated type in message templates that none of the strips + // above match. Two of them interpolate a name the caller never wrote: the parent OUTPUT + // type of an unknown argument (KnownArgumentNamesRule), which for a Pointer/Relation + // sub-selection is the TARGET class, and the enum type behind a Relation field's `order` + // argument (`Order`, src/GraphQL/loaders/parseClassTypes.js:311,332), which is + // reported by GraphQLEnumType itself rather than by a validation rule. Redact those + // identifiers for callers that are not allowed to introspect. + const setupRelationSchema = async _parseServer => { + const schemaController = await _parseServer.config.databaseController.loadSchema(); + await schemaController.addClassIfNotExists('SecretAuthor', { + name: { type: 'String' }, + }); + await schemaController.addClassIfNotExists('DiagShelf', { + authors: { type: 'Relation', targetClass: 'SecretAuthor' }, + }); + await resetGraphQLCache(); + }; + + it('should strip target class names from unknown-argument validation errors without master or maintenance key', async () => { + await setupPointerSchema(parseServer); + + try { + await apolloClient.query({ + query: gql` + query Leak { + diagBooks(where: {}) { + edges { + node { + writtenBy { + name(bogusArg: 1) + } + } + } + } + } + `, + }); + fail('should have thrown a validation error'); + } catch (e) { + const error = getReturnedError(e); + // Unknown argument "bogusArg" on field "SecretAuthor.name". + expect(error.message).not.toContain('SecretAuthor'); + expect(JSON.stringify(error)).not.toContain('SecretAuthor'); + } + }); + + it('should strip target class names from enum literal validation errors without master or maintenance key', async () => { + await setupRelationSchema(parseServer); + + try { + await apolloClient.query({ + query: gql` + query Leak { + diagShelves(where: {}) { + edges { + node { + authors(order: BOGUS) { + edges { + node { + id + } + } + } + } + } + } + } + `, + }); + fail('should have thrown a validation error'); + } catch (e) { + const error = getReturnedError(e); + // Value "BOGUS" does not exist in "SecretAuthorOrder" enum. + expect(error.message).not.toContain('SecretAuthor'); + expect(JSON.stringify(error)).not.toContain('SecretAuthor'); + } + }); + + it('should strip target class names from non-enum literal validation errors without master or maintenance key', async () => { + await setupRelationSchema(parseServer); + + try { + await apolloClient.query({ + query: gql` + query Leak { + diagShelves(where: {}) { + edges { + node { + authors(order: "BOGUS") { + edges { + node { + id + } + } + } + } + } + } + } + `, + }); + fail('should have thrown a validation error'); + } catch (e) { + const error = getReturnedError(e); + // Enum "SecretAuthorOrder" cannot represent non-enum value: "BOGUS". + expect(error.message).not.toContain('SecretAuthor'); + expect(JSON.stringify(error)).not.toContain('SecretAuthor'); + } + }); + + it('should keep caller-referenced enum type names in variable-coercion errors without master or maintenance key', async () => { + await setupRelationSchema(parseServer); + + try { + await apolloClient.query({ + query: gql` + query Leak($order: [SecretAuthorOrder!]) { + diagShelves(where: {}) { + edges { + node { + authors(order: $order) { + edges { + node { + id + } + } + } + } + } + } + } + `, + variables: { order: ['BOGUS'] }, + }); + fail('should have thrown a coercion error'); + } catch (e) { + const error = getReturnedError(e); + // The caller wrote SecretAuthorOrder in the operation text, so the enum name is not a + // disclosure and must be preserved to keep validation feedback useful. Sending a + // variable requires declaring its type, so the variable path can never name an enum + // the caller did not already write; only the literal paths above disclose. + expect(error.message).toContain('SecretAuthorOrder'); + } + }); + + it('should keep target class names in unknown-argument errors with master key', async () => { + await setupPointerSchema(parseServer); + + try { + await apolloClient.query({ + query: gql` + query Leak { + diagBooks(where: {}) { + edges { + node { + writtenBy { + name(bogusArg: 1) + } + } + } + } + } + `, + context: { + headers: { + 'X-Parse-Master-Key': 'test', + }, + }, + }); + fail('should have thrown a validation error'); + } catch (e) { + const error = getReturnedError(e); + expect(error.message).toContain('SecretAuthor'); + } + }); + + it('should keep target class names in enum errors with maintenance key', async () => { + await setupRelationSchema(parseServer); + + try { + await apolloClient.query({ + query: gql` + query Leak { + diagShelves(where: {}) { + edges { + node { + authors(order: BOGUS) { + edges { + node { + id + } + } + } + } + } + } + } + `, + context: { + headers: { + 'X-Parse-Maintenance-Key': 'test2', + }, + }, + }); + fail('should have thrown a validation error'); + } catch (e) { + const error = getReturnedError(e); + expect(error.message).toContain('SecretAuthorOrder'); + } + }); + + it('should keep target class names in enum errors when public introspection is enabled', async () => { + const parseServer = await reconfigureServer(); + await createGQLFromParseServer(parseServer, { graphQLPublicIntrospection: true }); + await setupRelationSchema(parseServer); + + try { + await apolloClient.query({ + query: gql` + query Leak { + diagShelves(where: {}) { + edges { + node { + authors(order: BOGUS) { + edges { + node { + id + } + } + } + } + } + } + } + `, + }); + fail('should have thrown a validation error'); + } catch (e) { + const error = getReturnedError(e); + expect(error.message).toContain('SecretAuthorOrder'); + } + }); + + it('should keep built-in enum type names in validation errors without master or maintenance key', async () => { + await setupRelationSchema(parseServer); + + try { + await apolloClient.query({ + query: gql` + query Leak { + diagShelves(where: {}, options: { readPreference: BOGUS }) { + edges { + node { + id + } + } + } + } + `, + }); + fail('should have thrown a validation error'); + } catch (e) { + const error = getReturnedError(e); + // Value "BOGUS" does not exist in "ReadPreference" enum. ReadPreference is a built-in + // enum, identical on every deployment and reachable from the `options` argument of + // every generated find query, so redacting it would degrade the message for no + // security gain. Guards the non-disclosing-name carve-out the enum templates rely on. + expect(error.message).toContain('ReadPreference'); + } + }); + + // The advisory also lists ProvidedRequiredArgumentsRule and the opposite branch of + // ScalarLeafsRule. Both templates are genuinely uncovered, but neither can name a class + // the caller did not already write in Parse Server's generated schema: no generated field + // carries a non-null argument naming a foreign class (a Relation field's find args are all + // nullable, src/GraphQL/loaders/parseClassTypes.js:332), and no generated leaf output type + // embeds a class name (leaf fields resolve to built-in scalars). Pinned rather than fixed. + it('should not disclose unreferenced class names in required-argument validation errors without master or maintenance key', async () => { + await setupPointerSchema(parseServer); + + try { + await apolloClient.mutate({ + mutation: gql` + mutation Leak { + createDiagBook { + diagBook { + id + } + } + } + `, + }); + fail('should have thrown a validation error'); + } catch (e) { + const error = getReturnedError(e); + // Field "createDiagBook" argument "input" of type "CreateDiagBookInput!" is required, + // but it was not provided. CreateDiagBookInput derives from the mutation name the + // caller wrote; SecretAuthor is the canary for a class name they did not write. + expect(error.message).not.toContain('SecretAuthor'); + expect(JSON.stringify(error)).not.toContain('SecretAuthor'); + } + }); + + it('should not disclose unreferenced class names in scalar-leaf selection validation errors without master or maintenance key', async () => { + await setupPointerSchema(parseServer); + + try { + await apolloClient.query({ + query: gql` + query Leak { + diagBooks(where: {}) { + edges { + node { + writtenBy { + name { + bogusSubField + } + } + } + } + } + } + `, + }); + fail('should have thrown a validation error'); + } catch (e) { + const error = getReturnedError(e); + // Field "name" must not have a selection since type "String" has no subfields. + expect(error.message).not.toContain('SecretAuthor'); + expect(JSON.stringify(error)).not.toContain('SecretAuthor'); + } + }); + }); diff --git a/src/GraphQL/ParseGraphQLSchema.js b/src/GraphQL/ParseGraphQLSchema.js index 5ecdd78de5..d5c3e3b41a 100644 --- a/src/GraphQL/ParseGraphQLSchema.js +++ b/src/GraphQL/ParseGraphQLSchema.js @@ -497,4 +497,4 @@ class ParseGraphQLSchema { } } -export { ParseGraphQLSchema }; +export { ParseGraphQLSchema, RESERVED_GRAPHQL_TYPE_NAMES }; diff --git a/src/GraphQL/ParseGraphQLServer.js b/src/GraphQL/ParseGraphQLServer.js index 4b431ddcdc..f98f2540a4 100644 --- a/src/GraphQL/ParseGraphQLServer.js +++ b/src/GraphQL/ParseGraphQLServer.js @@ -8,7 +8,8 @@ import { allowCrossDomain, handleParseErrors, handleParseHeaders, handleParseSes import requiredParameter from '../requiredParameter'; import { createComplexityValidationPlugin } from './helpers/queryComplexity'; import defaultLogger from '../logger'; -import { ParseGraphQLSchema } from './ParseGraphQLSchema'; +import { ParseGraphQLSchema, RESERVED_GRAPHQL_TYPE_NAMES } from './ParseGraphQLSchema'; +import { READ_PREFERENCE } from './loaders/defaultGraphQLTypes'; import ParseGraphQLController, { ParseGraphQLConfig } from '../Controllers/ParseGraphQLController'; @@ -82,6 +83,16 @@ const stripSchemaCoercionIdentifiers = message => ) : message; +// Type names that reveal nothing about THIS application's schema, so redacting them would cost +// message quality for no security gain. `RESERVED_GRAPHQL_TYPE_NAMES` covers the names the schema +// builder refuses to let a class generate, but Parse registers built-in types outside that list +// too. Of those, only an ENUM can reach the enum templates below, and the GraphQL layer defines +// exactly three enums: `CloudCodeFunction` (reserved), the per-class `Order` (the +// disclosure these templates exist to redact) and `ReadPreference`. Include the last one so that +// an invalid `options.readPreference` value still names its enum, just as `CloudCodeFunction` +// does — it is identical on every deployment and reachable from every generated find query. +const NON_DISCLOSING_TYPE_NAMES = new Set([...RESERVED_GRAPHQL_TYPE_NAMES, READ_PREFERENCE.name]); + // graphql-js also emits base coercion / validation messages that name a nested input // TYPE without a "Did you mean" clause, so neither strip above reaches them. For a // Pointer or Relation field the generated input type name embeds the pointer's TARGET @@ -95,49 +106,60 @@ const stripSchemaCoercionIdentifiers = message => // unavailable the identifier is redacted (fail closed). const stripSchemaTypeIdentifiers = (message, operationText) => { if (typeof message !== 'string') { return message; } - // A generated type identifier counts as "referenced" (and therefore not a disclosure) only if - // the caller wrote it as a whole token in the operation text. Tokenize the operation on - // non-identifier characters and compare exact tokens rather than building a RegExp from the - // captured name: this avoids substring false-matches (e.g. preserving "AuthorPointerInput" - // because the operation contains "SecretAuthorPointerInput") and any regex injection/ReDoS from - // an unusual captured name. GraphQL list/non-null wrappers ("[", "]", "!") are stripped from the - // captured name so e.g. "SecretAuthorPointerInput!" still matches "$x: SecretAuthorPointerInput!". - // When the operation text is unavailable the type is treated as not referenced (fail closed). + // A generated type identifier is kept (it is not a disclosure) if the caller wrote it as a + // whole token in the operation text, or if it is a non-disclosing name (above). Tokenize the + // operation on non-identifier characters and compare exact tokens rather than building a + // RegExp from the captured name: this avoids substring false-matches (e.g. preserving + // "AuthorPointerInput" because the operation contains "SecretAuthorPointerInput") and any + // regex injection/ReDoS from an unusual captured name. GraphQL list/non-null wrappers ("[", + // "]", "!") are stripped from the captured name so e.g. "SecretAuthorPointerInput!" still + // matches "$x: SecretAuthorPointerInput!". When the operation text is unavailable the type is + // treated as not referenced (fail closed). const referencedTokens = typeof operationText === 'string' ? new Set(operationText.split(/[^_A-Za-z0-9]+/).filter(Boolean)) : new Set(); - const isReferenced = typeName => referencedTokens.has(typeName.replace(/[[\]!]/g, '')); + // A non-disclosing type name (`CloudCodeFunction`, `ReadPreference`, `Viewer`, `PageInfo`, the + // built-in scalars, ...) is identical on every Parse Server deployment and cannot collide with + // a user class, since `ParseGraphQLSchema` rejects class names that would produce one. Echoing + // one therefore discloses nothing about THIS application's schema, so it is preserved like a + // caller-referenced name and the message stays useful. + const shouldKeepTypeName = typeName => { + const bareTypeName = typeName.replace(/[[\]!]/g, ''); + return NON_DISCLOSING_TYPE_NAMES.has(bareTypeName) || referencedTokens.has(bareTypeName); + }; return message // Input coercion / ValuesOfCorrectTypeRule (variables and inline literals). .replace(/Expected value of type "([^"]+)"/g, (match, typeName) => - isReferenced(typeName) ? match : 'Expected value of the correct type' + shouldKeepTypeName(typeName) ? match : 'Expected value of the correct type' ) .replace(/Expected type "([^"]+)" to be an object\./g, (match, typeName) => - isReferenced(typeName) ? match : 'Expected an object.' + shouldKeepTypeName(typeName) ? match : 'Expected an object.' ) .replace(/Expected non-nullable type "([^"]+)" not to be null\./g, (match, typeName) => - isReferenced(typeName) ? match : 'Expected a non-null value.' + shouldKeepTypeName(typeName) ? match : 'Expected a non-null value.' ) .replace(/ is not defined by type "([^"]+)"\./g, (match, typeName) => - isReferenced(typeName) ? match : ' is not defined.' + shouldKeepTypeName(typeName) ? match : ' is not defined.' ) // VariablesInAllowedPositionRule: the position type is the pointer/relation target // input type; the caller only wrote their own variable's declared type. .replace(/ used in position expecting type "([^"]+)"\./g, (match, typeName) => - isReferenced(typeName) ? match : ' used in position expecting a different type.' + shouldKeepTypeName(typeName) ? match : ' used in position expecting a different type.' ) // FieldsOnCorrectTypeRule: descending into a Pointer/Relation output field names its // target output object type. .replace(/Cannot query field ("[^"]*") on type "([^"]+)"\./g, (match, fieldName, typeName) => - isReferenced(typeName) ? match : `Cannot query field ${fieldName}.` + shouldKeepTypeName(typeName) ? match : `Cannot query field ${fieldName}.` ) // ScalarLeafsRule: selecting a Pointer/Relation output field with no sub-selection names // its target output object type. .replace( /Field ("[^"]*") of type "([^"]+)" must have a selection of subfields\./g, (match, fieldName, typeName) => - isReferenced(typeName) ? match : `Field ${fieldName} must have a selection of subfields.` + shouldKeepTypeName(typeName) + ? match + : `Field ${fieldName} must have a selection of subfields.` ) // PossibleFragmentSpreadsRule: an inline/named fragment on an incompatible type inside a // Pointer/Relation output field names the target output object type (the parent type). @@ -146,10 +168,37 @@ const stripSchemaTypeIdentifiers = (message, operationText) => { .replace( /objects of type "([^"]+)" can never be of type "([^"]+)"\./g, (match, parentType, fragType) => { - const parent = isReferenced(parentType) ? `type "${parentType}"` : 'the parent type'; - const frag = isReferenced(fragType) ? `type "${fragType}"` : 'the given type'; + const parent = shouldKeepTypeName(parentType) ? `type "${parentType}"` : 'the parent type'; + const frag = shouldKeepTypeName(fragType) ? `type "${fragType}"` : 'the given type'; return `objects of ${parent} can never be of ${frag}.`; } + ) + // KnownArgumentNamesRule: the argument's parent OUTPUT type. Inside a Pointer/Relation + // sub-selection that parent is the TARGET class, whose output type is named exactly the + // class name (`parseClassTypes.js`), so it discloses a class the caller never wrote — they + // only supplied the pointer field name. The type is embedded in a dotted "." + // token; keep the field name (the caller wrote it) and redact only the type half. Note the + // directive form ('... on directive "@name".') carries no type and is left alone. + .replace( + /Unknown argument ("[^"]*") on field "([^".]+)\.([^".]+)"\./g, + (match, argName, typeName, fieldName) => + shouldKeepTypeName(typeName) + ? match + : `Unknown argument ${argName} on field "${fieldName}".` + ) + // GraphQLEnumType.parseValue/parseLiteral: a Relation field's `order` argument is typed + // `[Order!]` (`parseClassTypes.js`), so an invalid enum literal names the target + // class. These messages come from the enum type itself rather than from a validation rule, + // which is why the ValuesOfCorrectTypeRule templates above do not reach them. The offending + // value is caller-supplied and is preserved; only the enum type name is redacted. + .replace(/Value ("[^"]*") does not exist in "([^"]+)" enum\./g, (match, value, typeName) => + shouldKeepTypeName(typeName) ? match : `Value ${value} does not exist in the enum.` + ) + // Sibling branches of the same enum type, reached when the literal is not an enum value at + // all ('cannot represent non-enum value: ...', 'cannot represent non-string value: ...', + // 'cannot represent value: ...'). Same disclosure, same redaction. + .replace(/Enum "([^"]+)" cannot represent /g, (match, typeName) => + shouldKeepTypeName(typeName) ? match : 'Enum cannot represent ' ); };