Skip to content

Commit d955f0e

Browse files
committed
Security: address CodeQL remarks
1 parent d347b3e commit d955f0e

File tree

7 files changed

+94
-17
lines changed

7 files changed

+94
-17
lines changed

packages/auth-kerberos/src/index.ts

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -165,12 +165,32 @@ export class KerberosAuth {
165165
*/
166166
extractToken(req: Request): string | null {
167167
const authHeader = req.headers.authorization;
168-
if (!authHeader) {
168+
// Limit header length to prevent potential issues with very long inputs
169+
if (!authHeader || authHeader.length > 8192) {
169170
return null;
170171
}
171172

172-
const match = authHeader.match(/^Negotiate\s+(.+)$/i);
173-
return match ? match[1] : null;
173+
// Use case-insensitive prefix check and manual whitespace handling
174+
// to avoid regex that might trigger ReDoS warnings
175+
const headerLower = authHeader.toLowerCase();
176+
if (!headerLower.startsWith("negotiate")) {
177+
return null;
178+
}
179+
180+
// Skip "negotiate" (9 chars) and whitespace
181+
let pos = 9;
182+
while (pos < authHeader.length) {
183+
const c = authHeader[pos];
184+
if (c !== " " && c !== "\t") break;
185+
pos++;
186+
}
187+
188+
// Must have at least one whitespace after "negotiate"
189+
if (pos === 9 || pos >= authHeader.length) {
190+
return null;
191+
}
192+
193+
return authHeader.substring(pos);
174194
}
175195

176196
/**

packages/issuer-cas/src/issuer.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -573,6 +573,12 @@ export class CASIssuer {
573573
return undefined;
574574
}
575575

576+
// Defensive type check - pgtUrl could be array from query params
577+
if (typeof pgtUrl !== "string") {
578+
this.log("warn", "Invalid pgtUrl type");
579+
return undefined;
580+
}
581+
576582
// Validate pgtUrl (must be HTTPS)
577583
if (!pgtUrl.startsWith("https://")) {
578584
this.log("warn", `Invalid pgtUrl (not HTTPS): ${pgtUrl}`);
@@ -698,6 +704,10 @@ export class CASIssuer {
698704
* Build redirect URL with ticket
699705
*/
700706
private buildRedirectUrl(service: string, ticket: string): string {
707+
// Defensive type check - service could be array from query params
708+
if (typeof service !== "string") {
709+
service = String(service);
710+
}
701711
const separator = service.includes("?") ? "&" : "?";
702712
return `${service}${separator}ticket=${ticket}`;
703713
}

packages/issuer-oidc/src/provider.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2068,6 +2068,11 @@ export class OIDCProvider {
20682068
* Check if a URI is potentially dangerous (XSS, injection)
20692069
*/
20702070
private isDangerousUri(uri: string): boolean {
2071+
// Limit URI length to prevent potential ReDoS (also rejects unreasonably long URIs)
2072+
if (!uri || uri.length > 8192) {
2073+
return true;
2074+
}
2075+
20712076
// Check for dangerous schemes
20722077
const dangerousSchemes = /^\s*(javascript|vbscript|data):/i;
20732078
if (dangerousSchemes.test(uri)) {

packages/issuer-oidc/src/router.ts

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -476,12 +476,26 @@ async function handleEndSession(
476476
): Promise<void> {
477477
const { provider, renderLogout } = options;
478478

479-
const postLogoutRedirectUri = (req.query.post_logout_redirect_uri ||
480-
req.body?.post_logout_redirect_uri) as string;
481-
const idTokenHint = (req.query.id_token_hint ||
482-
req.body?.id_token_hint) as string;
483-
const clientId = (req.query.client_id || req.body?.client_id) as string;
484-
const state = (req.query.state || req.body?.state) as string;
479+
// Helper to safely extract string param (could be array from query string)
480+
const getStringParam = (
481+
value: string | string[] | undefined,
482+
): string | undefined => {
483+
if (Array.isArray(value)) return value[0];
484+
return value;
485+
};
486+
487+
const postLogoutRedirectUri = getStringParam(
488+
req.query.post_logout_redirect_uri as string | string[] | undefined,
489+
) || getStringParam(req.body?.post_logout_redirect_uri);
490+
const idTokenHint = getStringParam(
491+
req.query.id_token_hint as string | string[] | undefined,
492+
) || getStringParam(req.body?.id_token_hint);
493+
const clientId = getStringParam(
494+
req.query.client_id as string | string[] | undefined,
495+
) || getStringParam(req.body?.client_id);
496+
const state = getStringParam(
497+
req.query.state as string | string[] | undefined,
498+
) || getStringParam(req.body?.state);
485499

486500
// Validate logout request
487501
const validation = provider.validateLogoutRequest({

packages/lib-cas/src/utils.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@ export function isServiceUrlValid(
1010
serviceUrl: string,
1111
allowedPatterns: string[],
1212
): boolean {
13+
// Defensive type check - ensure serviceUrl is a string (could be array from query params)
14+
if (typeof serviceUrl !== "string") {
15+
return false;
16+
}
1317
if (!serviceUrl || !allowedPatterns || allowedPatterns.length === 0) {
1418
return false;
1519
}
@@ -87,6 +91,10 @@ export function appendQueryParam(
8791
key: string,
8892
value: string,
8993
): string {
94+
// Defensive type check
95+
if (typeof url !== "string") {
96+
url = String(url);
97+
}
9098
const separator = url.includes("?") ? "&" : "?";
9199
return `${url}${separator}${encodeURIComponent(key)}=${encodeURIComponent(value)}`;
92100
}
@@ -218,7 +226,11 @@ export function buildSamlValidateUrl(
218226
* Normalize CAS server URL (remove trailing slash)
219227
*/
220228
export function normalizeCasServerUrl(url: string): string {
221-
return url.replace(/\/+$/, "");
229+
// Remove trailing slashes without regex to avoid ReDoS warnings
230+
while (url.endsWith("/")) {
231+
url = url.slice(0, -1);
232+
}
233+
return url;
222234
}
223235

224236
/**

packages/lib-cas/src/xml-parser.ts

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,12 @@
44

55
import { SAMLValidateRequest, CASValidateResult } from "./types";
66

7+
/**
8+
* Maximum XML input size to prevent ReDoS attacks
9+
* 1MB should be more than enough for any valid CAS/SAML response
10+
*/
11+
const MAX_XML_SIZE = 1024 * 1024;
12+
713
/**
814
* Simple XML tag extraction (without full XML parser dependency)
915
*/
@@ -60,7 +66,7 @@ function extractAttribute(
6066
export function parseSamlValidateRequest(
6167
xml: string,
6268
): SAMLValidateRequest | null {
63-
if (!xml) return null;
69+
if (!xml || xml.length > MAX_XML_SIZE) return null;
6470

6571
// Extract ticket from AssertionArtifact
6672
const ticket = extractTagContent(xml, "AssertionArtifact");
@@ -95,11 +101,11 @@ function unescapeXml(str: string): string {
95101
* Parse CAS serviceValidate response
96102
*/
97103
export function parseServiceValidateResponse(xml: string): CASValidateResult {
98-
if (!xml) {
104+
if (!xml || xml.length > MAX_XML_SIZE) {
99105
return {
100106
success: false,
101107
code: "INVALID_RESPONSE",
102-
message: "Empty response",
108+
message: !xml ? "Empty response" : "Response too large",
103109
};
104110
}
105111

@@ -199,11 +205,11 @@ export function parseValidateResponse(response: string): CASValidateResult {
199205
* Parse SAML validate response (for CAS SP)
200206
*/
201207
export function parseSamlValidateResponse(xml: string): CASValidateResult {
202-
if (!xml) {
208+
if (!xml || xml.length > MAX_XML_SIZE) {
203209
return {
204210
success: false,
205211
code: "INVALID_RESPONSE",
206-
message: "Empty response",
212+
message: !xml ? "Empty response" : "Response too large",
207213
};
208214
}
209215

@@ -273,11 +279,11 @@ export function parseProxyResponse(
273279
):
274280
| { success: true; proxyTicket: string }
275281
| { success: false; code: string; message: string } {
276-
if (!xml) {
282+
if (!xml || xml.length > MAX_XML_SIZE) {
277283
return {
278284
success: false,
279285
code: "INVALID_RESPONSE",
280-
message: "Empty response",
286+
message: !xml ? "Empty response" : "Response too large",
281287
};
282288
}
283289

packages/lib-saml/src/utils.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,11 +317,21 @@ export function parseQueryString(query: string): Record<string, string> {
317317
return params;
318318
}
319319

320+
/**
321+
* Maximum SOAP envelope size to prevent ReDoS attacks
322+
*/
323+
const MAX_SOAP_SIZE = 1024 * 1024; // 1MB
324+
320325
/**
321326
* Extract SAML message from SOAP envelope
322327
* Supports both LogoutRequest and LogoutResponse
323328
*/
324329
export function extractSamlFromSoap(soapEnvelope: string): string | null {
330+
// Limit input size to prevent ReDoS
331+
if (!soapEnvelope || soapEnvelope.length > MAX_SOAP_SIZE) {
332+
return null;
333+
}
334+
325335
// Try to extract LogoutRequest
326336
let match = soapEnvelope.match(
327337
/<(?:samlp:|)[Ll]ogout[Rr]equest[\s\S]*?<\/(?:samlp:|)[Ll]ogout[Rr]equest>/,

0 commit comments

Comments
 (0)