eduvia/workavia-mail-front
11
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fixup! TF-4224 Reduce ReDoS vulnerability

Browse Source
This commit is contained in:
Dang Dat
2025-12-24 14:41:37 +07:00
committed by Dat H. Pham
parent 2b909aeb1c
commit 86dec27d67
6 changed files with 424 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/lib/presentation/utils/html_transformer/base/dom_transformer.dart
+3 -2
View File
@@ -9,6 +9,8 @@ abstract class DomTransformer {
const DomTransformer();
static final _backgroundImageRegex = RegExp(r'''\bbackground-image\s*:\s*url\(\s*['"]?([^' ")]+)['"]?\s*\)''');
/// Uses the `DOM` [document] to transform the `document`.
///
/// All changes will be visible to subsequent transformers.
@@ -27,8 +29,7 @@ abstract class DomTransformer {
Tuple2<String, String>? findImageUrlFromStyleTag(String style) {
try {
final regExp = RegExp(r'''\bbackground-image\s*:\s*url\(\s*['"]?([^' ")]+)['"]?\s*\)''');
final match = regExp.firstMatch(style);
final match = _backgroundImageRegex.firstMatch(style);
if (match == null) {
return null;
}
core/lib/presentation/utils/html_transformer/dom/normalize_line_height_in_style_transformer.dart
+2 -1
View File
@@ -7,6 +7,7 @@ class NormalizeLineHeightInStyleTransformer extends DomTransformer {
const NormalizeLineHeightInStyleTransformer();
static const _removableLineHeights = ['1px', '100%'];
static final _multipleSpacesRegex = RegExp(r' {2,}');
@override
Future<void> process({
@@ -30,7 +31,7 @@ class NormalizeLineHeightInStyleTransformer extends DomTransformer {
var updatedStyle = originalStyle.replaceAll(pattern, '').trim();
// Remove extra spaces (>=2 spaces → 1 space)
updatedStyle = updatedStyle.replaceAll(RegExp(r' {2,}'), ' ');
updatedStyle = updatedStyle.replaceAll(_multipleSpacesRegex, ' ');
if (updatedStyle != originalStyle) {
if (updatedStyle.isEmpty) {
core/lib/utils/html/html_utils.dart
+16 -14
View File
@@ -24,6 +24,17 @@ class HtmlUtils {
static final random = Random();
static final htmlUnescape = HtmlUnescape();
// ReDoS-safe regex patterns as const
static final _htmlStartTagRegex = RegExp(r'<[a-zA-Z][^>\s]*[^>]*>');
static final _htmlEndTagRegex = RegExp(r'</[a-zA-Z][^>]{0,128}>');
static final _whitespaceNormalizationRegex = RegExp(r'\s+');
static final _urlRegex = RegExp(
r'''(?:https?://|ftp://|mailto:|file://|www\.)[^\s<.]+(?:\.[^\s<.]+)*(?<![.,:;!?"')\]])''',
caseSensitive: false,
multiLine: true,
);
static final _protocolRegex = RegExp(r'^(?:https?|ftp|mailto|file)');
static const removeLineHeight1px = (
script: '''
document.querySelectorAll('[style*="line-height"]').forEach(el => {
@@ -700,8 +711,8 @@ class HtmlUtils {
}
static String addQuoteToggle(String htmlString) {
final likelyHtml = htmlString.contains(RegExp(r'<[a-zA-Z][^>\s]*[^>]*>')) && // Contains a start tag
htmlString.contains(RegExp(r'</[a-zA-Z][^>]{0,128}>')); // Contains an end tag
final likelyHtml = htmlString.contains(_htmlStartTagRegex) && // Contains a start tag
htmlString.contains(_htmlEndTagRegex); // Contains an end tag
if (!likelyHtml) {
return htmlString; // Not likely HTML, return original
@@ -858,7 +869,7 @@ class HtmlUtils {
cleaned = cleaned.replaceAll(tagRegex, ' ');
// Normalize whitespace
cleaned = cleaned.replaceAll(RegExp(r'\s+'), ' ').trim();
cleaned = cleaned.replaceAll(_whitespaceNormalizationRegex, ' ').trim();
return cleaned;
}
@@ -870,13 +881,7 @@ class HtmlUtils {
if (container == null) return htmlString;
final urlRegex = RegExp(
r'''(?:https?://|ftp://|mailto:|file://|www\.)[^\s<.]+(?:\.[^\s<.]+)*(?<![.,:;!?"')\]])''',
caseSensitive: false,
multiLine: true,
);
_processNode(container, urlRegex);
_processNode(container, _urlRegex);
return container.innerHtml;
} catch (e) {
@@ -899,9 +904,6 @@ class HtmlUtils {
};
static void _processNode(dom.Node node, RegExp urlRegex) {
// 1. Anchored with ^ to fail instantly if the start doesn't match
// 2. Grouped for clarity
final protocolRegex = RegExp(r'^(?:https?|ftp|mailto|file)');
for (var child in node.nodes.toList()) {
// Skip if node or parent node is in tag to skip
final parentTag = child.parent?.localName;
@@ -932,7 +934,7 @@ class HtmlUtils {
nodes.add(dom.Text(url));
} else {
// Normalize href
final href = url.startsWith(protocolRegex)
final href = url.startsWith(_protocolRegex)
? url
: 'https://$url';
core/lib/utils/string_convert.dart
+14 -19
View File
@@ -12,6 +12,15 @@ class StringConvert {
// Change the pattern to include newlines directly to avoid the .replaceAll() step
static const String emailSeparatorPattern = r'[ ,;\n\r\t]+';
// ReDoS-safe regex patterns as static final
static final _base64ValidationRegex = RegExp(r'^[A-Za-z0-9+/=]+$');
static final _mdSeparatorRegex = RegExp(
r'^\|?(?:[ \t]*:?-+:?[ \t]*\|)+[ \t]*:?-+:?[ \t]*\|?$',
multiLine: false,
);
static final _asciiArtRegex = RegExp(r'[+\-|/\\=]');
static final _namedAddressRegex = RegExp(r'''(?:(?:"([^"]+)"|'([^']+)')\s*)?<([^>]+)>''');
static String? writeEmptyToNull(String text) {
if (text.isEmpty) return null;
return text;
@@ -40,7 +49,7 @@ class StringConvert {
// 2. Base64 Check - Using a non-regex check first is faster
if (input.length % 4 == 0 && !input.contains(' ')) {
// Only run regex if basic length/whitespace checks pass
if (RegExp(r'^[A-Za-z0-9+/=]+$').hasMatch(input)) {
if (_base64ValidationRegex.hasMatch(input)) {
try {
input = utf8.decode(base64.decode(input));
} catch (_) {
@@ -132,25 +141,14 @@ class StringConvert {
text.split('\n').where((line) => line.trim().isNotEmpty).toList();
if (lines.length < 2) return false;
// 1. Hardened Markdown Separator Regex
// We use [ \t]* instead of \s to be specific and avoid newline issues
// We ensure the dash sequence is clearly delimited
final mdSeparatorRegex = RegExp(
r'^\|?(?:[ \t]*:?-+:?[ \t]*\|)+[ \t]*:?-+:?[ \t]*\|?$',
multiLine: false);
// 2. Optimized ASCII Check
// We use hasMatch instead of split().length for better performance
final asciiArtRegex = RegExp(r'[+\-|/\\=]');
bool isMarkdown = false;
bool allLinesHaveAscii = true;
for (final line in lines) {
if (!isMarkdown && mdSeparatorRegex.hasMatch(line)) {
if (!isMarkdown && _mdSeparatorRegex.hasMatch(line)) {
isMarkdown = true;
}
if (allLinesHaveAscii && !asciiArtRegex.hasMatch(line)) {
if (allLinesHaveAscii && !_asciiArtRegex.hasMatch(line)) {
allLinesHaveAscii = false;
}
// Early exit if we found a table but also know it's not ASCII art
@@ -166,8 +164,7 @@ class StringConvert {
input = Uri.decodeComponent(input);
}
if (input.length % 4 == 0 &&
input.contains(RegExp(r'^[A-Za-z0-9+/=]+$'))) {
if (input.length % 4 == 0 && _base64ValidationRegex.hasMatch(input)) {
try {
input = utf8.decode(base64.decode(input));
} catch (_) {}
@@ -176,11 +173,9 @@ class StringConvert {
input = input.replaceAll('\n', ' ');
final results = <NamedAddress>[];
final pattern = RegExp(r'''(?:(?:"([^"]+)"|'([^']+)')\s*)?<([^>]+)>''');
int currentIndex = 0;
// Use a for-in loop directly on the iterable to save memory
for (final match in pattern.allMatches(input)) {
for (final match in _namedAddressRegex.allMatches(input)) {
if (match.start > currentIndex) {
final between = input.substring(currentIndex, match.start);
results.addAll(_splitPlainAddresses(between, emailSeparatorPattern));
core/test/utils/redos_vulnerability_test.dart
+384
View File
@@ -0,0 +1,384 @@
import 'package:core/presentation/utils/html_transformer/dom/image_transformers.dart';
import 'package:core/utils/html/html_utils.dart';
import 'package:core/utils/string_convert.dart';
import 'package:flutter_test/flutter_test.dart';
/// Tests to verify that ReDoS (Regular Expression Denial of Service) vulnerabilities
/// have been properly patched. These tests use inputs that would cause catastrophic
/// backtracking in the old regex patterns.
void main() {
group('ReDoS Vulnerability Patches -', () {
group('DomTransformer._backgroundImageRegex', () {
const transformer = ImageTransformer();
test('should handle very long background-image style without freezing', () {
// Create a malicious input that would cause catastrophic backtracking
// in the old pattern: r'background-image:\s*url\(([^)]+)\).*?'
final maliciousStyle = 'background-image: url(${'a' * 10000})';
final stopwatch = Stopwatch()..start();
final result = transformer.findImageUrlFromStyleTag(maliciousStyle);
stopwatch.stop();
// Should complete within a reasonable time (< 100ms)
expect(stopwatch.elapsedMilliseconds, lessThan(100),
reason: 'Regex should not cause catastrophic backtracking');
expect(result, isNotNull);
});
test('should handle style with many nested parentheses', () {
final style = 'background-image: url(${'(' * 1000}example.jpg${')' * 1000})';
final stopwatch = Stopwatch()..start();
final result = transformer.findImageUrlFromStyleTag(style);
stopwatch.stop();
expect(stopwatch.elapsedMilliseconds, lessThan(100));
expect(result, isNotNull);
});
test('should reject malformed background-image patterns quickly', () {
// Input with many whitespace and incomplete patterns
final maliciousInput = 'background-image:${' ' * 10000}url(';
final stopwatch = Stopwatch()..start();
final result = transformer.findImageUrlFromStyleTag(maliciousInput);
stopwatch.stop();
expect(stopwatch.elapsedMilliseconds, lessThan(100));
expect(result, isNull);
});
test('should correctly parse valid background-image patterns', () {
const validPatterns = [
'background-image: url(example.com/image.jpg)',
'background-image: url("example.com/image.jpg")',
"background-image: url('example.com/image.jpg')",
'background-image:url(example.com/image.jpg)',
];
for (final pattern in validPatterns) {
final result = transformer.findImageUrlFromStyleTag(pattern);
expect(result, isNotNull, reason: 'Should parse: $pattern');
expect(result!.value2, contains('example.com/image.jpg'));
}
});
});
group('NormalizeLineHeightInStyleTransformer', () {
test('should handle very long style strings efficiently', () {
final longStyle = 'color: red; ' * 10000 + 'line-height: 1px;';
final stopwatch = Stopwatch()..start();
final normalized = longStyle.replaceAll(
RegExp(r'line-height\s*:\s*(?:1px|100%)\s*;?', caseSensitive: false),
'',
);
stopwatch.stop();
expect(stopwatch.elapsedMilliseconds, lessThan(100));
expect(normalized, isNot(contains('line-height')));
});
test('should handle many consecutive spaces efficiently', () {
final manySpaces = 'text${' ' * 10000}more text';
final stopwatch = Stopwatch()..start();
final normalized = manySpaces.replaceAll(RegExp(r' {2,}'), ' ');
stopwatch.stop();
expect(stopwatch.elapsedMilliseconds, lessThan(100));
expect(normalized, equals('text more text'));
});
});
group('HtmlUtils regex patterns', () {
test('_htmlStartTagRegex should handle malicious tag patterns', () {
// Old pattern: r'<[a-zA-Z][^>]*>' could cause issues with many attributes
final maliciousTag = '<div${'a' * 10000}>';
final stopwatch = Stopwatch()..start();
HtmlUtils.addQuoteToggle(maliciousTag);
stopwatch.stop();
expect(stopwatch.elapsedMilliseconds, lessThan(100));
});
test('_htmlEndTagRegex should have bounded quantifier', () {
// Old pattern had unbounded quantifier: r'</[a-zA-Z][^>]*>'
// New pattern has max 128 chars: r'</[a-zA-Z][^>]{0,128}>'
final longEndTag = '</div${'x' * 200}>';
final stopwatch = Stopwatch()..start();
HtmlUtils.addQuoteToggle(longEndTag);
stopwatch.stop();
expect(stopwatch.elapsedMilliseconds, lessThan(100));
});
test('_urlRegex should handle long URLs efficiently', () {
// Old pattern: r'(?:(?:https?:\/\/)|...)(?!\.)(?!.*\.\.)([^\s<]+[^<.,:;\"\'\)\]\s!?])'
// had potential for catastrophic backtracking
final longUrl = 'https://${'a' * 10000}.com${'b' * 10000}';
final stopwatch = Stopwatch()..start();
HtmlUtils.wrapPlainTextLinks('<p>$longUrl</p>');
stopwatch.stop();
expect(stopwatch.elapsedMilliseconds, lessThan(200));
});
test('_urlRegex should correctly match valid URLs', () {
const validUrls = [
'https://example.com',
'http://test.org',
'ftp://files.com',
'mailto:test@example.com',
'file:///path/to/file',
'www.example.com',
];
for (final url in validUrls) {
final html = '<p>Visit $url today</p>';
final result = HtmlUtils.wrapPlainTextLinks(html);
expect(result, contains('<a href='), reason: 'Should wrap: $url');
}
});
test('_urlRegex should reject malformed URLs quickly', () {
const malformedUrls = [
'http://.example.com', // Leading dot
'https://..example.com', // Double dots
'www..com', // Double dots
];
for (final url in malformedUrls) {
final html = '<p>Visit $url today</p>';
final stopwatch = Stopwatch()..start();
HtmlUtils.wrapPlainTextLinks(html);
stopwatch.stop();
expect(stopwatch.elapsedMilliseconds, lessThan(100),
reason: 'Should process quickly: $url');
}
});
test('extractPlainText should handle long HTML with escaped tags', () {
// Test that escaped tag processing doesn't cause issues
final longHtml = '${'&lt;div&gt;' * 1000}content${'&lt;/div&gt;' * 1000}';
final stopwatch = Stopwatch()..start();
final result = HtmlUtils.extractPlainText(longHtml);
stopwatch.stop();
expect(stopwatch.elapsedMilliseconds, lessThan(500));
expect(result, contains('content'));
});
});
group('StringConvert regex patterns', () {
test('_base64ValidationRegex should handle long invalid base64', () {
// Test with a very long string that looks like base64 but isn't
final longInvalidBase64 = 'A' * 10000 + '!'; // Not valid base64
final stopwatch = Stopwatch()..start();
StringConvert.extractEmailAddress(longInvalidBase64);
stopwatch.stop();
expect(stopwatch.elapsedMilliseconds, lessThan(100));
});
test('_mdSeparatorRegex should handle malicious markdown separators', () {
// Old pattern: r'^\|?(\s*:?-+:?\s*\|)+(\s*:?-+:?\s*)\|?$'
// Could cause issues with many repeating patterns
final maliciousMarkdown = '|${':---:' * 1000}|';
final stopwatch = Stopwatch()..start();
StringConvert.isTextTable('$maliciousMarkdown\n$maliciousMarkdown');
stopwatch.stop();
expect(stopwatch.elapsedMilliseconds, lessThan(100));
});
test('_asciiArtRegex should efficiently check long text', () {
final longText = 'a' * 10000 + '+';
final stopwatch = Stopwatch()..start();
StringConvert.isTextTable('$longText\n$longText');
stopwatch.stop();
expect(stopwatch.elapsedMilliseconds, lessThan(100));
});
test('_namedAddressRegex should handle many quoted addresses', () {
// Test with many named addresses
final manyAddresses = List.generate(
1000,
(i) => '"User $i" <user$i@example.com>',
).join(', ');
final stopwatch = Stopwatch()..start();
final result = StringConvert.extractNamedAddresses(manyAddresses);
stopwatch.stop();
expect(stopwatch.elapsedMilliseconds, lessThan(500));
expect(result.length, equals(1000));
});
test('extractEmailAddress should handle malicious separator patterns', () {
// Old pattern could have issues with many mixed separators
final maliciousInput = 'email@test.com${', ; ' * 10000}another@test.com';
final stopwatch = Stopwatch()..start();
final result = StringConvert.extractEmailAddress(maliciousInput);
stopwatch.stop();
expect(stopwatch.elapsedMilliseconds, lessThan(100));
expect(result, contains('email@test.com'));
expect(result, contains('another@test.com'));
});
test('isTextTable should correctly identify valid tables', () {
const validTables = [
'''
| Header 1 | Header 2 |
|----------|----------|
| Cell 1 | Cell 2 |
''',
'''
+--------+--------+
| Cell 1 | Cell 2 |
+--------+--------+
''',
];
for (final table in validTables) {
expect(StringConvert.isTextTable(table), isTrue,
reason: 'Should identify as table');
}
});
test('isTextTable should correctly reject non-tables', () {
const nonTables = [
'Just plain text',
'Single line with | pipes',
'',
' \n ',
];
for (final text in nonTables) {
expect(StringConvert.isTextTable(text), isFalse,
reason: 'Should not identify as table: $text');
}
});
});
group('AppUtils._emailLocalhostRegex', () {
test('should handle very long email addresses efficiently', () {
final longEmail = '${'a' * 10000}@localhost';
final stopwatch = Stopwatch()..start();
// Note: We can't directly test AppUtils.isEmailLocalhost in core tests
// But we can test the regex pattern
final regex = RegExp(
r'^(?:"[^"\r\n]+"|[^<>()[\]\\.,;:\s@"]+(?:\.[^<>()[\]\\.,;:\s@"]+)*)@localhost$',
);
final result = regex.hasMatch(longEmail);
stopwatch.stop();
expect(stopwatch.elapsedMilliseconds, lessThan(100));
expect(result, isTrue);
});
test('should correctly match valid localhost emails', () {
final regex = RegExp(
r'^(?:"[^"\r\n]+"|[^<>()[\]\\.,;:\s@"]+(?:\.[^<>()[\]\\.,;:\s@"]+)*)@localhost$',
);
const validEmails = [
'user@localhost',
'user.name@localhost',
'"User Name"@localhost',
];
for (final email in validEmails) {
expect(regex.hasMatch(email.trim()), isTrue,
reason: 'Should match: $email');
}
});
test('should reject invalid localhost emails', () {
final regex = RegExp(
r'^(?:"[^"\r\n]+"|[^<>()[\]\\.,;:\s@"]+(?:\.[^<>()[\]\\.,;:\s@"]+)*)@localhost$',
);
const invalidEmails = [
'user@localhost.com',
'@localhost',
'user@',
'not-an-email',
];
for (final email in invalidEmails) {
expect(regex.hasMatch(email.trim()), isFalse,
reason: 'Should not match: $email');
}
});
});
group('Performance stress tests', () {
test('all regex patterns should handle edge case inputs within time limit', () {
// Create various edge case inputs
final edgeCases = [
'x' * 100000, // Very long string
'(' * 1000 + ')' * 1000, // Many nested parentheses
' ' * 100000, // Many spaces
'\n' * 10000, // Many newlines
'|||||||' * 10000, // Many pipes
'-------' * 10000, // Many dashes
];
for (final input in edgeCases) {
final stopwatch = Stopwatch()..start();
// Test various functions with edge case input
try {
StringConvert.extractEmailAddress(input);
StringConvert.isTextTable('$input\n$input');
HtmlUtils.extractPlainText(input);
} catch (e) {
// It's OK if these fail, we're just testing performance
}
stopwatch.stop();
expect(stopwatch.elapsedMilliseconds, lessThan(1000),
reason: 'Should process edge case within 1 second');
}
});
});
group('Regex initialization optimization', () {
test('regex patterns should be reused, not recreated', () {
// This test verifies that regex patterns are static and reused
// We can't directly test if they're the same object, but we can
// verify performance doesn't degrade with multiple calls
const iterations = 1000;
final stopwatch = Stopwatch()..start();
for (var i = 0; i < iterations; i++) {
StringConvert.isTextTable('| a | b |\n|---|---|\n| c | d |');
HtmlUtils.wrapPlainTextLinks('<p>https://example.com</p>');
StringConvert.extractEmailAddress('test@example.com, test2@example.com');
}
stopwatch.stop();
// If regex patterns were being recreated each time, this would be much slower
expect(stopwatch.elapsedMilliseconds, lessThan(500),
reason: 'Multiple calls should be fast due to regex reuse');
});
});
});
}
lib/main/utils/app_utils.dart
+5 -3
View File
@@ -14,6 +14,10 @@ import 'package:tmail_ui_user/main/universal_import/html_stub.dart' as html;
class AppUtils {
const AppUtils._();
static final _emailLocalhostRegex = RegExp(
r'^(?:"[^"\r\n]+"|[^<>()[\]\\.,;:\s@"]+(?:\.[^<>()[\]\\.,;:\s@"]+)*)@localhost$',
);
static Future<void> loadEnvFile() async {
await dotenv.load(fileName: AppConfig.envFileName);
final mapEnvData = Map<String, String>.from(dotenv.env);
@@ -53,9 +57,7 @@ class AppUtils {
static bool isEmailLocalhost(String email) {
final normalized = email.trim();
return RegExp(
r'^(?:"[^"\r\n]+"|[^<>()[\]\\.,;:\s@"]+(?:\.[^<>()[\]\\.,;:\s@"]+)*)@localhost$'
).hasMatch(normalized);
return _emailLocalhostRegex.hasMatch(normalized);
}
static void copyEmailAddressToClipboard(BuildContext context, String emailAddress) {