diff --git a/core/lib/utils/string_convert.dart b/core/lib/utils/string_convert.dart index b0a164a80..d105a6233 100644 --- a/core/lib/utils/string_convert.dart +++ b/core/lib/utils/string_convert.dart @@ -21,10 +21,23 @@ class StringConvert { ); static final _asciiArtRegex = RegExp(r'[+\-|/\\=]'); static final _namedAddressRegex = RegExp(r'''(?:(?:"([^"]+)"|'([^']+)')\s*)?<([^>]+)>'''); + static final _emailLocalhostRegex = RegExp( + r'^(?:"[^"\r\n]+"|[^<>()[\]\\.,;:\s@"]+(?:\.[^<>()[\]\\.,;:\s@"]+)*)@localhost$', + ); @visibleForTesting static RegExp get base64ValidationRegex => _base64ValidationRegex; + @visibleForTesting + static RegExp get emailLocalhostRegex => _emailLocalhostRegex; + + /// Checks if the given email address is a localhost email + /// Returns true if the email ends with @localhost + static bool isEmailLocalhost(String email) { + final normalized = email.trim(); + return _emailLocalhostRegex.hasMatch(normalized); + } + static String? writeEmptyToNull(String text) { if (text.isEmpty) return null; return text; diff --git a/core/test/utils/redos_vulnerability_test.dart b/core/test/utils/redos_vulnerability_test.dart index 75ab412df..1ef1f1755 100644 --- a/core/test/utils/redos_vulnerability_test.dart +++ b/core/test/utils/redos_vulnerability_test.dart @@ -1,7 +1,13 @@ +import 'package:core/data/network/dio_client.dart'; import 'package:core/presentation/utils/html_transformer/dom/image_transformers.dart'; +import 'package:core/presentation/utils/html_transformer/dom/normalize_line_height_in_style_transformer.dart'; import 'package:core/utils/html/html_utils.dart'; import 'package:core/utils/string_convert.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:html/parser.dart' as html_parser; +import 'package:mockito/mockito.dart'; + +class MockDioClient extends Mock implements DioClient {} /// Tests to verify that ReDoS (Regular Expression Denial of Service) vulnerabilities /// have been properly patched. These tests use inputs that would cause catastrophic @@ -65,30 +71,130 @@ void main() { }); }); - group('NormalizeLineHeightInStyleTransformer tests regex patterns in isolation', () { - test('should handle very long style strings efficiently', () { + group('NormalizeLineHeightInStyleTransformer', () { + late NormalizeLineHeightInStyleTransformer transformer; + late MockDioClient mockDioClient; + + setUp(() { + transformer = const NormalizeLineHeightInStyleTransformer(); + mockDioClient = MockDioClient(); + }); + + test('should handle very long style strings efficiently', () async { + // Create HTML with very long style attribute final longStyle = 'color: red; ' * 10000 + 'line-height: 1px;'; + final htmlContent = '
Content
'; + final document = html_parser.parse(htmlContent); final stopwatch = Stopwatch()..start(); - final normalized = longStyle.replaceAll( - RegExp(r'line-height\s*:\s*(?:1px|100%)\s*;?', caseSensitive: false), - '', + await transformer.process( + document: document, + dioClient: mockDioClient, ); stopwatch.stop(); expect(stopwatch.elapsedMilliseconds, lessThan(100)); - expect(normalized, isNot(contains('line-height'))); + + // Verify the transformer removed line-height + final divElement = document.querySelector('div'); + final resultStyle = divElement?.attributes['style'] ?? ''; + expect(resultStyle, isNot(contains('line-height'))); }); - test('should handle many consecutive spaces efficiently', () { - final manySpaces = 'text${' ' * 10000}more text'; + test('should handle many consecutive spaces efficiently', () async { + // Create HTML with many spaces in style that includes line-height + final manySpaces = 'color:${' ' * 10000}red; line-height: 1px;'; + final htmlContent = '

Text

'; + final document = html_parser.parse(htmlContent); final stopwatch = Stopwatch()..start(); - final normalized = manySpaces.replaceAll(RegExp(r' {2,}'), ' '); + await transformer.process( + document: document, + dioClient: mockDioClient, + ); stopwatch.stop(); expect(stopwatch.elapsedMilliseconds, lessThan(100)); - expect(normalized, equals('text more text')); + + // After removing line-height, spaces should be normalized + final pElement = document.querySelector('p'); + final resultStyle = pElement?.attributes['style'] ?? ''; + expect(resultStyle, isNot(contains('line-height'))); + // Multiple consecutive spaces should be normalized to single space + expect(resultStyle, isNot(matches(r' {2,}'))); + }); + + test('should remove line-height: 1px and 100% patterns', () async { + final testCases = [ + '
Test 1px
', + '
Test 100%
', + '
No spaces
', + '
Case insensitive
', + ]; + + for (final htmlContent in testCases) { + final document = html_parser.parse(htmlContent); + await transformer.process( + document: document, + dioClient: mockDioClient, + ); + + final divElement = document.querySelector('div'); + final resultStyle = divElement?.attributes['style'] ?? ''; + expect(resultStyle, isNot(contains('line-height')), + reason: 'Should remove line-height from: $htmlContent'); + } + }); + + test('should preserve other line-height values', () async { + final testCases = [ + '
Test 1.5
', + '
Test 20px
', + '
Test normal
', + ]; + + for (final htmlContent in testCases) { + final document = html_parser.parse(htmlContent); + await transformer.process( + document: document, + dioClient: mockDioClient, + ); + + final divElement = document.querySelector('div'); + final resultStyle = divElement?.attributes['style'] ?? ''; + expect(resultStyle, contains('line-height'), + reason: 'Should preserve line-height from: $htmlContent'); + } + }); + + test('should remove style attribute when empty after transformation', () async { + const htmlContent = '
Empty after
'; + final document = html_parser.parse(htmlContent); + + await transformer.process( + document: document, + dioClient: mockDioClient, + ); + + final divElement = document.querySelector('div'); + expect(divElement?.attributes.containsKey('style'), isFalse, + reason: 'Should remove empty style attribute'); + }); + + test('should normalize multiple spaces after removing line-height', () async { + const htmlContent = '
Multi-space
'; + final document = html_parser.parse(htmlContent); + + await transformer.process( + document: document, + dioClient: mockDioClient, + ); + + final divElement = document.querySelector('div'); + final resultStyle = divElement?.attributes['style'] ?? ''; + expect(resultStyle, isNot(contains(' '))); // No double spaces + expect(resultStyle, contains('color: red')); + expect(resultStyle, contains('font-size: 12px')); }); }); @@ -366,35 +472,53 @@ void main() { expect(result, contains('another@test.com')); }); - test('Should handle emails with special characters and edge cases', () { - final specialCharEmails = [ - // Valid special characters in local part - 'user+tag@example.com', - 'user.name@example.com', - 'user_name@example.com', - 'user-name@example.com', - 'user123@example.com', - '123user@example.com', - // International domain - 'user@example.co.uk', - 'user@sub.domain.example.com', - // Quoted local part (valid but rare) + test('Should handle emails with special characters - valid cases', () { + // Valid emails that should be extracted successfully + final validEmailInputs = { + 'user+tag@example.com': 'user+tag@example.com', + 'user.name@example.com': 'user.name@example.com', + 'user_name@example.com': 'user_name@example.com', + 'user-name@example.com': 'user-name@example.com', + 'user123@example.com': 'user123@example.com', + '123user@example.com': '123user@example.com', + 'user@example.co.uk': 'user@example.co.uk', + 'user@sub.domain.example.com': 'user@sub.domain.example.com', + 'a.b.c.d@example.com': 'a.b.c.d@example.com', + 'user@例え.jp': 'user@例え.jp', + '${'a' * 64}@example.com': '${'a' * 64}@example.com', + 'Contact us at support@example.com for help': 'support@example.com', + 'Multiple emails: alice@test.com, bob@test.org': contains('alice@test.com'), + }; + + for (final entry in validEmailInputs.entries) { + final input = entry.key; + final expectedMatch = entry.value; + + final stopwatch = Stopwatch()..start(); + final result = StringConvert.extractEmailAddress(input); + stopwatch.stop(); + + expect(stopwatch.elapsedMilliseconds, lessThan(100), + reason: 'Should process quickly: $input'); + + if (expectedMatch is String) { + expect(result, contains(expectedMatch), + reason: 'Should extract "$expectedMatch" from: $input'); + } else { + expect(result, expectedMatch, + reason: 'Should match expected pattern from: $input'); + } + } + }); + + test('Should handle quoted email addresses', () { + // Quoted email addresses have special handling in email specs + const quotedEmails = [ '"user@name"@example.com', '"user name"@example.com', - // Edge cases with dots - 'a.b.c.d@example.com', - 'user.@example.com', // Invalid but should handle gracefully - '.user@example.com', // Invalid but should handle gracefully - // Unicode/international emails (if supported) - 'user@例え.jp', - // Very long local part - '${'a' * 64}@example.com', - // Mixed with other text - 'Contact us at support@example.com for help', - 'Multiple emails: alice@test.com, bob@test.org', ]; - for (final input in specialCharEmails) { + for (final input in quotedEmails) { final stopwatch = Stopwatch()..start(); final result = StringConvert.extractEmailAddress(input); stopwatch.stop(); @@ -403,11 +527,39 @@ void main() { expect(stopwatch.elapsedMilliseconds, lessThan(100), reason: 'Should process quickly: $input'); - // If it looks like a valid email, should extract something - if (input.contains('@') && !input.startsWith('.') && !input.endsWith('.')) { - expect(result, isNotEmpty, - reason: 'Should extract email from: $input'); - } + // Should extract something (even if split) - no crash + expect(result, isA>(), + reason: 'Should return a list for: $input'); + expect(result, isNotEmpty, + reason: 'Should extract something from: $input'); + } + }); + + test('Should gracefully handle invalid or edge case emails', () { + // Invalid emails or edge cases - should handle gracefully without crashing + const edgeCaseInputs = [ + 'user.@example.com', // Dot before @ + '.user@example.com', // Leading dot + 'user..name@example.com', // Consecutive dots + '@example.com', // Missing local part + 'user@', // Missing domain + 'user', // No @ symbol + '', // Empty string + ' ', // Only whitespace + ]; + + for (final input in edgeCaseInputs) { + final stopwatch = Stopwatch()..start(); + final result = StringConvert.extractEmailAddress(input); + stopwatch.stop(); + + // Performance check - should handle gracefully without hanging + expect(stopwatch.elapsedMilliseconds, lessThan(100), + reason: 'Should process quickly even for invalid: $input'); + + // Result should be a list (even if empty) - no crash + expect(result, isA>(), + reason: 'Should return a list for: $input'); } }); @@ -446,17 +598,12 @@ void main() { }); }); - group('AppUtils._emailLocalhostRegex', () { - final localhostRegex = RegExp( - r'^(?:"[^"\r\n]+"|[^<>()[\]\\.,;:\s@"]+(?:\.[^<>()[\]\\.,;:\s@"]+)*)@localhost$', - ); + group('StringConvert.isEmailLocalhost', () { 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 result = localhostRegex.hasMatch(longEmail); + final result = StringConvert.isEmailLocalhost(longEmail); stopwatch.stop(); expect(stopwatch.elapsedMilliseconds, lessThan(100)); @@ -471,7 +618,7 @@ void main() { ]; for (final email in validEmails) { - expect(localhostRegex.hasMatch(email.trim()), isTrue, + expect(StringConvert.isEmailLocalhost(email), isTrue, reason: 'Should match: $email'); } }); @@ -485,7 +632,7 @@ void main() { ]; for (final email in invalidEmails) { - expect(localhostRegex.hasMatch(email.trim()), isFalse, + expect(StringConvert.isEmailLocalhost(email), isFalse, reason: 'Should not match: $email'); } }); diff --git a/lib/main/utils/app_utils.dart b/lib/main/utils/app_utils.dart index de17233f6..5a7044697 100644 --- a/lib/main/utils/app_utils.dart +++ b/lib/main/utils/app_utils.dart @@ -14,10 +14,6 @@ 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 loadEnvFile() async { await dotenv.load(fileName: AppConfig.envFileName); final mapEnvData = Map.from(dotenv.env); @@ -56,8 +52,7 @@ class AppUtils { static TextDirection getCurrentDirection(BuildContext context) => Directionality.maybeOf(context) ?? TextDirection.ltr; static bool isEmailLocalhost(String email) { - final normalized = email.trim(); - return _emailLocalhostRegex.hasMatch(normalized); + return StringConvert.isEmailLocalhost(email); } static void copyEmailAddressToClipboard(BuildContext context, String emailAddress) {