Skip to content

Commit a62a46f

Browse files
committed
fix(security): resolve high CodeQL alerts
Arthas command builders escaped quote characters without first escaping existing backslashes, allowing crafted values to neutralize the quote escaping. Escape both layers and consistently encode dynamic OGNL string values. SQL Server XE fields were parsed as 64-bit values before narrowing to int. Parse session IDs and error numbers directly at the destination integer width to avoid overflow during conversion.
1 parent c51c32d commit a62a46f

4 files changed

Lines changed: 27 additions & 14 deletions

File tree

arthas/ui-src/src/pages/ArthasJulLoggingPanel.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ function javaString(s: string): string {
298298
}
299299

300300
function escapeSingleQuotes(s: string): string {
301-
return s.replace(/'/g, "\\'");
301+
return s.replace(/\\/g, "\\\\").replace(/'/g, "\\'");
302302
}
303303

304304
function throwIfOgnlError(results: unknown[]): void {

arthas/ui-src/src/pages/ArthasMBeanTab.tsx

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ export function ArthasMBeanTab({ sessionId }: { sessionId: string }) {
155155
queryKey: ["arthas", sessionId, "mbean", "detail", selected ?? ""],
156156
enabled: !!selected,
157157
queryFn: async () => {
158-
const cmd = `mbean '${selected!.replace(/'/g, "\\'")}'`;
158+
const cmd = `mbean ${quoteArg(selected!)}`;
159159
const { results } = await execArthas(sessionId, cmd);
160160
for (const r of results as Array<{ mbeanAttribute?: Record<string, MBeanAttribute[]> }>) {
161161
if (r?.mbeanAttribute) return r.mbeanAttribute[selected!] ?? [];
@@ -169,7 +169,7 @@ export function ArthasMBeanTab({ sessionId }: { sessionId: string }) {
169169
enabled: !!selected,
170170
staleTime: 60_000,
171171
queryFn: async () => {
172-
const cmd = `mbean -m '${selected!.replace(/'/g, "\\'")}'`;
172+
const cmd = `mbean -m ${quoteArg(selected!)}`;
173173
const { results } = await execArthas(sessionId, cmd);
174174
for (const r of results as Array<{ mbeanMetadata?: Record<string, MBeanMetadata> }>) {
175175
if (r?.mbeanMetadata) return r.mbeanMetadata[selected!] ?? null;
@@ -608,7 +608,15 @@ function coerceLiteral(raw: string, type: string): string {
608608
return /^-?\d+(\.\d+)?$/.test(raw.trim()) ? raw.trim() : "0.0";
609609
}
610610
// Strings and everything else: quote as Java string literal.
611-
return `"${raw.replace(/\\/g, "\\\\").replace(/"/g, '\\"')}"`;
611+
return javaString(raw);
612+
}
613+
614+
function javaString(value: string): string {
615+
return `"${value.replace(/\\/g, "\\\\").replace(/"/g, '\\"')}"`;
616+
}
617+
618+
function quoteArg(value: string): string {
619+
return `'${value.replace(/\\/g, "\\\\").replace(/'/g, "\\'")}'`;
612620
}
613621

614622
async function writeMBeanAttribute(
@@ -621,11 +629,11 @@ async function writeMBeanAttribute(
621629
const literal = coerceLiteral(value, attributeType);
622630
const expr = [
623631
`(#server=@java.lang.management.ManagementFactory@getPlatformMBeanServer(),`,
624-
` #name=new javax.management.ObjectName("${objectName.replace(/"/g, '\\"')}"),`,
625-
` #attr=new javax.management.Attribute("${attribute}", ${literal}),`,
632+
` #name=new javax.management.ObjectName(${javaString(objectName)}),`,
633+
` #attr=new javax.management.Attribute(${javaString(attribute)}, ${literal}),`,
626634
` #server.setAttribute(#name, #attr))`,
627635
].join("");
628-
const { results } = await execArthas(sessionId, `ognl '${expr.replace(/'/g, "\\'")}'`);
636+
const { results } = await execArthas(sessionId, `ognl ${quoteArg(expr)}`);
629637
throwIfOgnlError(results);
630638
}
631639

@@ -636,15 +644,15 @@ async function invokeMBeanOperation(
636644
rawParams: string[],
637645
): Promise<unknown> {
638646
const literals = op.signature.map((p, i) => coerceLiteral(rawParams[i] ?? "", p.type));
639-
const types = op.signature.map((p) => `"${p.type}"`);
647+
const types = op.signature.map((p) => javaString(p.type));
640648
const expr = [
641649
`(#server=@java.lang.management.ManagementFactory@getPlatformMBeanServer(),`,
642-
` #name=new javax.management.ObjectName("${objectName.replace(/"/g, '\\"')}"),`,
650+
` #name=new javax.management.ObjectName(${javaString(objectName)}),`,
643651
` #args=new Object[]{${literals.join(", ")}},`,
644652
` #sig=new String[]{${types.join(", ")}},`,
645-
` #server.invoke(#name, "${op.name}", #args, #sig))`,
653+
` #server.invoke(#name, ${javaString(op.name)}, #args, #sig))`,
646654
].join("");
647-
const { results } = await execArthas(sessionId, `ognl '${expr.replace(/'/g, "\\'")}'`);
655+
const { results } = await execArthas(sessionId, `ognl ${quoteArg(expr)}`);
648656
throwIfOgnlError(results);
649657
for (const r of results as Array<{ value?: unknown; type?: string }>) {
650658
if (r?.type === "ognl" && "value" in r) return r.value;

arthas/ui-src/src/pages/ArthasProfilerTab.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,7 @@ function buildProfilerCommand(
433433

434434
function quoteArg(value: string): string {
435435
if (/^[A-Za-z0-9_./:=+@,%*-]+$/.test(value)) return value;
436-
return `'${value.replace(/'/g, "\\'")}'`;
436+
return `'${value.replace(/\\/g, "\\\\").replace(/'/g, "\\'")}'`;
437437
}
438438

439439
function extractFlamegraphURL(results: unknown[]): string | undefined {

sql-server/internal/xetrace/parser.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,9 +130,9 @@ func applyField(e *Event, f rawXMLField) {
130130
case "username":
131131
e.Username = val
132132
case "session_id":
133-
e.SessionID = int(parseInt64(val))
133+
e.SessionID = parseInt(val)
134134
case "error_number":
135-
e.ErrorNumber = int(parseInt64(val))
135+
e.ErrorNumber = parseInt(val)
136136
case "message":
137137
e.ErrorMessage = val
138138
}
@@ -143,6 +143,11 @@ func parseInt64(s string) int64 {
143143
return n
144144
}
145145

146+
func parseInt(s string) int {
147+
n, _ := strconv.Atoi(s)
148+
return n
149+
}
150+
146151
func firstN(s string, n int) string {
147152
if len(s) <= n {
148153
return s

0 commit comments

Comments
 (0)