Skip to content

Commit 3965512

Browse files
authored
fix: codeql issues (#16870)
This PR fixes a couple of 'high' severity CodeQL issues: 1. `urlJoin` ReDOS - Instead of using regexes we walk the start/end of each string and cut appropriately 2. viem error parsing and formatting ReDOS - this was problematic because we were potentially applying regexes on tens of KB of body. It now only renders the first 1KB of text and the last 1KB of text based on the fact `data: 0x...` being in the middle and not really useful. Fix TMNT-279
2 parents 45f9163 + abd3498 commit 3965512

File tree

3 files changed

+39
-188
lines changed

3 files changed

+39
-188
lines changed

yarn-project/ethereum/src/utils.ts

Lines changed: 13 additions & 185 deletions
Original file line numberDiff line numberDiff line change
@@ -181,41 +181,19 @@ export function formatViemError(error: any, abi: Abi = ErrorsAbi): FormattedViem
181181
return new FormattedViemError(error.message, (error as any)?.metaMessages);
182182
}
183183

184-
// Extract the actual error message and highlight it for clarity
185-
let formattedRes = extractAndFormatRequestBody(error?.message || String(error));
186-
187-
let errorDetail = '';
188-
// Look for specific details in known locations
189-
if (error) {
190-
// Check for details property which often has the most specific error message
191-
if (typeof error.details === 'string' && error.details) {
192-
errorDetail = error.details;
193-
}
194-
// Check for shortMessage which is often available in Viem errors
195-
else if (typeof error.shortMessage === 'string' && error.shortMessage) {
196-
errorDetail = error.shortMessage;
197-
}
198-
}
199-
200-
// If we found a specific error detail, format it clearly
201-
if (errorDetail) {
202-
// Look for key sections of the formatted result to replace with highlighted error
203-
let replaced = false;
204-
205-
// Try to find the Details: section
206-
const detailsMatch = formattedRes.match(/Details: ([^\n]+)/);
207-
if (detailsMatch) {
208-
formattedRes = formattedRes.replace(detailsMatch[0], `Details: *${errorDetail}*`);
209-
replaced = true;
210-
}
211-
212-
// If we didn't find a Details section, add the error at the beginning
213-
if (!replaced) {
214-
formattedRes = `Error: *${errorDetail}*\n\n${formattedRes}`;
215-
}
216-
}
217-
218-
return new FormattedViemError(formattedRes.replace(/\\n/g, '\n'), error?.metaMessages);
184+
const body = String(error);
185+
const length = body.length;
186+
// LogExplorer can only render up to 2500 characters in it's summary view. Try to keep the whole message below this number
187+
// Limit the error to 2000 chacaters in order to allow code higher up to decorate this error with extra details (up to 500 characters)
188+
if (length > 2000) {
189+
const chunk = 950;
190+
const truncated = length - 2 * chunk;
191+
return new FormattedViemError(
192+
body.slice(0, chunk) + `...${truncated} characters truncated...` + body.slice(-1 * chunk),
193+
);
194+
}
195+
196+
return new FormattedViemError(body);
219197
}
220198

221199
function stripAbis(obj: any) {
@@ -241,156 +219,6 @@ function stripAbis(obj: any) {
241219
});
242220
}
243221

244-
function extractAndFormatRequestBody(message: string): string {
245-
// First check if message is extremely large and contains very large hex strings
246-
if (message.length > 50000) {
247-
message = replaceHexStrings(message, { minLength: 10000, truncateLength: 200 });
248-
}
249-
250-
// Add a specific check for RPC calls with large params
251-
if (message.includes('"method":"eth_sendRawTransaction"')) {
252-
message = replaceHexStrings(message, {
253-
pattern: /"params":\s*\[\s*"(0x[a-fA-F0-9]{1000,})"\s*\]/g,
254-
transform: hex => `"params":["${truncateHex(hex, 200)}"]`,
255-
});
256-
}
257-
258-
// First handle Request body JSON
259-
const requestBodyRegex = /Request body: ({[\s\S]*?})\n/g;
260-
let result = message.replace(requestBodyRegex, (match, body) => {
261-
return `Request body: ${formatRequestBody(body)}\n`;
262-
});
263-
264-
// Then handle Arguments section
265-
const argsRegex = /((?:Request |Estimate Gas )?Arguments:[\s\S]*?(?=\n\n|$))/g;
266-
result = result.replace(argsRegex, section => {
267-
const lines = section.split('\n');
268-
const processedLines = lines.map(line => {
269-
// Check if line contains a colon followed by content
270-
const colonIndex = line.indexOf(':');
271-
if (colonIndex !== -1) {
272-
const [prefix, content] = [line.slice(0, colonIndex + 1), line.slice(colonIndex + 1).trim()];
273-
// If content contains a hex string, truncate it
274-
if (content.includes('0x')) {
275-
const processedContent = replaceHexStrings(content);
276-
return `${prefix} ${processedContent}`;
277-
}
278-
}
279-
return line;
280-
});
281-
return processedLines.join('\n');
282-
});
283-
284-
// Finally, catch any remaining hex strings in the message
285-
result = replaceHexStrings(result);
286-
287-
return result;
288-
}
289-
290-
function truncateHex(hex: string, length = 100) {
291-
if (!hex || typeof hex !== 'string') {
292-
return hex;
293-
}
294-
if (!hex.startsWith('0x')) {
295-
return hex;
296-
}
297-
if (hex.length <= length * 2) {
298-
return hex;
299-
}
300-
// For extremely large hex strings, use more aggressive truncation
301-
if (hex.length > 10000) {
302-
return `${hex.slice(0, length)}...<${hex.length - length * 2} chars omitted>...${hex.slice(-length)}`;
303-
}
304-
return `${hex.slice(0, length)}...${hex.slice(-length)}`;
305-
}
306-
307-
function replaceHexStrings(
308-
text: string,
309-
options: {
310-
minLength?: number;
311-
maxLength?: number;
312-
truncateLength?: number;
313-
pattern?: RegExp;
314-
transform?: (hex: string) => string;
315-
} = {},
316-
): string {
317-
const {
318-
minLength = 10,
319-
maxLength = Infinity,
320-
truncateLength = 100,
321-
pattern,
322-
transform = hex => truncateHex(hex, truncateLength),
323-
} = options;
324-
325-
const hexRegex = pattern ?? new RegExp(`(0x[a-fA-F0-9]{${minLength},${maxLength}})`, 'g');
326-
return text.replace(hexRegex, match => transform(match));
327-
}
328-
329-
function formatRequestBody(body: string) {
330-
try {
331-
// Special handling for eth_sendRawTransaction
332-
if (body.includes('"method":"eth_sendRawTransaction"')) {
333-
try {
334-
const parsed = JSON.parse(body);
335-
if (parsed.params && Array.isArray(parsed.params) && parsed.params.length > 0) {
336-
// These are likely large transaction hex strings
337-
parsed.params = parsed.params.map((param: any) => {
338-
if (typeof param === 'string' && param.startsWith('0x') && param.length > 1000) {
339-
return truncateHex(param, 200);
340-
}
341-
return param;
342-
});
343-
}
344-
return JSON.stringify(parsed, null, 2);
345-
} catch {
346-
// If specific parsing fails, fall back to regex-based truncation
347-
return replaceHexStrings(body, {
348-
pattern: /"params":\s*\[\s*"(0x[a-fA-F0-9]{1000,})"\s*\]/g,
349-
transform: hex => `"params":["${truncateHex(hex, 200)}"]`,
350-
});
351-
}
352-
}
353-
354-
// For extremely large request bodies, use simple truncation instead of parsing
355-
if (body.length > 50000) {
356-
const jsonStart = body.indexOf('{');
357-
const jsonEnd = body.lastIndexOf('}');
358-
if (jsonStart >= 0 && jsonEnd > jsonStart) {
359-
return replaceHexStrings(body, { minLength: 10000, truncateLength: 200 });
360-
}
361-
}
362-
363-
const parsed = JSON.parse(body);
364-
365-
// Process the entire request body
366-
const processed = processParams(parsed);
367-
return JSON.stringify(processed, null, 2);
368-
} catch {
369-
// If JSON parsing fails, do a simple truncation of any large hex strings
370-
return replaceHexStrings(body, { minLength: 1000, truncateLength: 150 });
371-
}
372-
}
373-
374-
// Recursively process all parameters that might contain hex strings
375-
function processParams(obj: any): any {
376-
if (Array.isArray(obj)) {
377-
return obj.map(item => processParams(item));
378-
}
379-
if (typeof obj === 'object' && obj !== null) {
380-
const result: any = {};
381-
for (const [key, value] of Object.entries(obj)) {
382-
result[key] = processParams(value);
383-
}
384-
return result;
385-
}
386-
if (typeof obj === 'string') {
387-
if (obj.startsWith('0x')) {
388-
return truncateHex(obj);
389-
}
390-
}
391-
return obj;
392-
}
393-
394222
export function tryGetCustomErrorName(err: any) {
395223
try {
396224
// See https://viem.sh/docs/contract/simulateContract#handling-custom-errors

yarn-project/foundation/src/string/index.test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,11 @@ describe('string', () => {
66
expect(urlJoin('http://example.com', 'foo', 'bar')).toBe('http://example.com/foo/bar');
77
});
88

9-
it('removes duplicate slashes', () => {
10-
expect(urlJoin('http://example.com/', '/foo/', '/bar/')).toBe('http://example.com/foo/bar');
9+
it.each([
10+
[['http://example.com/', '/foo/', '/bar/'], 'http://example.com/foo/bar'],
11+
[['http://example.com/', '', '//', '///', '////foo//', '//bar////', 'baz'], 'http://example.com/foo/bar/baz'],
12+
])('removes duplicate slashes', (parts, url) => {
13+
expect(urlJoin(...parts)).toBe(url);
1114
});
1215
});
1316

yarn-project/foundation/src/string/index.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,5 +39,25 @@ export function isoDate(date?: Date) {
3939
}
4040

4141
export function urlJoin(...args: string[]): string {
42-
return args.map(arg => arg.replace(/\/+$/, '').replace(/^\/+/, '')).join('/');
42+
const processed = [];
43+
for (const arg of args) {
44+
if (arg.length === 0) {
45+
continue;
46+
}
47+
48+
let start = 0;
49+
let end = arg.length - 1;
50+
51+
while (start <= end && arg[start] === '/') {
52+
start++;
53+
}
54+
while (end >= start && arg[end] === '/') {
55+
end--;
56+
}
57+
58+
if (start < end) {
59+
processed.push(arg.slice(start, end + 1));
60+
}
61+
}
62+
return processed.join('/');
4363
}

0 commit comments

Comments
 (0)