Skip to content

Commit 51b3899

Browse files
author
MStarRobotics
committed
fix(webhook): resolve CodeQL security warnings CWE-134, CWE-400, CWE-770
- Add rate limiting to /github-webhook (100 req/min per IP) - Validate required headers (X-GitHub-Event, X-GitHub-Delivery) before processing - Use structured logging instead of format strings to prevent CWE-134 - Restrict raw body parser to application/json only - Improve error messages for missing headers Fixes #44 (Use of externally-controlled format string) Fixes #43 (Missing rate limiting)
1 parent d7eb975 commit 51b3899

File tree

1 file changed

+33
-16
lines changed

1 file changed

+33
-16
lines changed

server/index.js

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,24 @@ app.use(cors(corsOptions));
4646
app.use(helmet());
4747
app.disable('x-powered-by');
4848

49+
const createRateLimiter = (config) =>
50+
rateLimit({
51+
standardHeaders: true,
52+
legacyHeaders: false,
53+
message: { error: 'Too many requests. Please try again later.' },
54+
...config,
55+
});
56+
4957
// GitHub Webhook endpoint (raw body + optional signature verification)
5058
// Payload URL when deployed: https://<your-host>/github-webhook
51-
app.use('/github-webhook', express.raw({ type: '*/*', limit: '200kb' }));
59+
const webhookRateLimit = createRateLimiter({
60+
windowMs: 60 * 1000, // 1 minute
61+
max: 100, // 100 requests per window per IP
62+
skipSuccessfulRequests: false,
63+
});
64+
65+
app.use('/github-webhook', express.raw({ type: 'application/json', limit: '200kb' }));
66+
5267
const verifyGithubSignature = (secret, payloadBuffer, signatureHeader) => {
5368
if (!secret || !signatureHeader) return true; // allow when not configured
5469
const expected = 'sha256=' + createHmac('sha256', secret).update(payloadBuffer).digest('hex');
@@ -59,11 +74,16 @@ const verifyGithubSignature = (secret, payloadBuffer, signatureHeader) => {
5974
}
6075
};
6176

62-
app.post('/github-webhook', (req, res) => {
63-
const event = req.header('X-GitHub-Event') ?? 'unknown';
64-
const delivery = req.header('X-GitHub-Delivery') ?? 'n/a';
77+
app.post('/github-webhook', webhookRateLimit, (req, res) => {
78+
const event = req.header('X-GitHub-Event');
79+
const delivery = req.header('X-GitHub-Delivery');
6580
const signature = req.header('X-Hub-Signature-256');
6681

82+
if (!event || !delivery) {
83+
res.status(400).json({ error: 'Missing GitHub webhook headers' });
84+
return;
85+
}
86+
6787
const isValid = verifyGithubSignature(GITHUB_WEBHOOK_SECRET, req.body, signature);
6888
if (!isValid) {
6989
res.status(401).json({ error: 'Invalid signature' });
@@ -73,27 +93,24 @@ app.post('/github-webhook', (req, res) => {
7393
try {
7494
// Best-effort parse; keep raw for signature
7595
const payload = typeof req.body === 'string' ? JSON.parse(req.body) : JSON.parse(req.body.toString('utf8'));
76-
console.log(`[GitHub Webhook] ${event} (${delivery})`, {
77-
repository: payload?.repository?.full_name,
78-
action: payload?.action,
96+
const repoName = payload?.repository?.full_name || 'unknown';
97+
const action = payload?.action || 'n/a';
98+
// Log structured data to avoid format string vulnerability
99+
console.log('[GitHub Webhook]', {
100+
event,
101+
delivery,
102+
repository: repoName,
103+
action,
79104
});
80105
} catch (e) {
81-
console.warn('Received webhook but failed to parse JSON payload');
106+
console.warn('Received webhook but failed to parse JSON payload:', e instanceof Error ? e.message : String(e));
82107
}
83108
res.status(200).json({ ok: true });
84109
});
85110

86111
// JSON body parser for the rest of the API
87112
app.use(express.json({ limit: '50kb' }));
88113

89-
const createRateLimiter = (config) =>
90-
rateLimit({
91-
standardHeaders: true,
92-
legacyHeaders: false,
93-
message: { error: 'Too many requests. Please try again later.' },
94-
...config,
95-
});
96-
97114
const pendingNonces = new Map();
98115
const walletDirectory = new Map();
99116
const userDirectory = new Map();

0 commit comments

Comments
 (0)