mirror of
https://github.com/FoggedLens/deflock.git
synced 2026-07-14 15:57:20 +02:00
fix GraphQL query injection in /sponsors/github
The username query param was interpolated directly into the GitHub GraphQL
query string (user(login: "${username}")) sent with the server token, and
the route only validated it as a string, so a value containing a quote could
break out of the string literal and alter the query document. Impact depends
on the token's scope; at minimum it allows tampering with the query structure.
Two changes:
- Pass the username as a $login GraphQL variable so it can never alter the
query document (the actual fix).
- Validate username against GitHub's username format at the route as
defense-in-depth, rejecting malformed input before it reaches the API.
Adds a regression test for the query builder (bun test). No change in
returned data for valid usernames.
This commit is contained in:
committed by
Jeremy Knows
parent
235f4ce87c
commit
3eceedb9c6
+2
-1
@@ -208,7 +208,8 @@ const start = async () => {
|
|||||||
querystring: {
|
querystring: {
|
||||||
type: 'object',
|
type: 'object',
|
||||||
properties: {
|
properties: {
|
||||||
username: { type: 'string', default: 'frillweeman' },
|
// GitHub usernames are 1-39 chars of letters, digits, and hyphens.
|
||||||
|
username: { type: 'string', pattern: '^[A-Za-z0-9-]{1,39}$', default: 'frillweeman' },
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
response: {
|
response: {
|
||||||
|
|||||||
@@ -0,0 +1,36 @@
|
|||||||
|
import { describe, it, expect } from 'bun:test';
|
||||||
|
import { buildSponsorsQuery } from './GithubClient';
|
||||||
|
|
||||||
|
describe('buildSponsorsQuery', () => {
|
||||||
|
it('parameterizes the username instead of interpolating it', () => {
|
||||||
|
const { query, variables } = buildSponsorsQuery('frillweeman');
|
||||||
|
expect(query).toContain('query($login: String!)');
|
||||||
|
expect(query).toContain('user(login: $login)');
|
||||||
|
expect(variables).toEqual({ login: 'frillweeman' });
|
||||||
|
});
|
||||||
|
|
||||||
|
it('never places the raw username inside the query string', () => {
|
||||||
|
const { query } = buildSponsorsQuery('octocat');
|
||||||
|
// The query is fixed and only references the $login variable — the caller's
|
||||||
|
// value must not appear in the query text itself.
|
||||||
|
expect(query).not.toContain('octocat');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('keeps GraphQL-breaking input confined to the variables (injection guard)', () => {
|
||||||
|
// A username containing a quote used to break out of the interpolated
|
||||||
|
// string literal. It must now travel only in `variables.login`.
|
||||||
|
const hostile = '") { viewer { login } } #';
|
||||||
|
const { query, variables } = buildSponsorsQuery(hostile);
|
||||||
|
expect(variables.login).toBe(hostile);
|
||||||
|
expect(query).not.toContain(hostile);
|
||||||
|
expect(query).not.toContain('viewer');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('requests sponsorships and the expected sponsor fields', () => {
|
||||||
|
const { query } = buildSponsorsQuery('frillweeman');
|
||||||
|
expect(query).toContain('sponsorshipsAsMaintainer(first: 100)');
|
||||||
|
for (const field of ['login', 'name', 'avatarUrl', 'url']) {
|
||||||
|
expect(query).toContain(field);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -17,10 +17,18 @@ export type Sponsor = Static<typeof SponsorSchema>;
|
|||||||
|
|
||||||
export const SponsorsResponseSchema = Type.Array(SponsorSchema);
|
export const SponsorsResponseSchema = Type.Array(SponsorSchema);
|
||||||
|
|
||||||
|
// Pass the username as a GraphQL variable rather than interpolating it into the
|
||||||
|
// query string, so a value containing quotes or other GraphQL syntax cannot
|
||||||
|
// break out of the string literal and alter the query.
|
||||||
|
export const buildSponsorsQuery = (username: string): { query: string; variables: { login: string } } => ({
|
||||||
|
query: `query($login: String!) { user(login: $login) { sponsorshipsAsMaintainer(first: 100) { nodes { sponsor { login name avatarUrl url } } } } }`,
|
||||||
|
variables: { login: username },
|
||||||
|
});
|
||||||
|
|
||||||
export class GithubClient {
|
export class GithubClient {
|
||||||
async getSponsors(username: string): Promise<Sponsor[]> {
|
async getSponsors(username: string): Promise<Sponsor[]> {
|
||||||
const query = `query { user(login: "${username}") { sponsorshipsAsMaintainer(first: 100) { nodes { sponsor { login name avatarUrl url } } } } }`;
|
const { query, variables } = buildSponsorsQuery(username);
|
||||||
const body = JSON.stringify({ query, variables: '' });
|
const body = JSON.stringify({ query, variables });
|
||||||
const response = await fetch(graphQLEndpoint, {
|
const response = await fetch(graphQLEndpoint, {
|
||||||
method: 'POST',
|
method: 'POST',
|
||||||
headers: {
|
headers: {
|
||||||
|
|||||||
Reference in New Issue
Block a user