diff --git a/core/lib/core.dart b/core/lib/core.dart index 6e5d7965b..59ef23830 100644 --- a/core/lib/core.dart +++ b/core/lib/core.dart @@ -6,6 +6,7 @@ export 'presentation/extensions/url_extension.dart'; export 'presentation/extensions/capitalize_extension.dart'; export 'presentation/extensions/list_extensions.dart'; export 'domain/extensions/datetime_extension.dart'; +export 'presentation/extensions/html_extension.dart'; // Utils export 'presentation/utils/theme_utils.dart'; @@ -44,3 +45,6 @@ export 'data/network/dio_client.dart'; export 'presentation/state/success.dart'; export 'presentation/state/failure.dart'; export 'presentation/state/app_state.dart'; + +// Validator +export 'presentation/validator/html_message_purifier.dart'; diff --git a/core/lib/presentation/extensions/html_extension.dart b/core/lib/presentation/extensions/html_extension.dart new file mode 100644 index 000000000..83db272e5 --- /dev/null +++ b/core/lib/presentation/extensions/html_extension.dart @@ -0,0 +1,15 @@ + +extension HtmlExtension on String { + + String removeFontSizeZeroPixel() => replaceAll(RegExp('font-size:0px;'), ''); + + String removeHeightZeroPixel() => replaceAll(RegExp('height:0px;'), ''); + + String removeWidthZeroPixel() => replaceAll(RegExp('width:0px;'), ''); + + String removeMaxWidthZeroPixel() => replaceAll(RegExp('max-width:0px;'), ''); + + String removeMaxHeightZeroPixel() => replaceAll(RegExp('max-height:0px;'), ''); + + String changeStyleBackground() => replaceAll(RegExp('background:'), 'background-color:'); +} diff --git a/core/lib/presentation/validator/html_message_purifier.dart b/core/lib/presentation/validator/html_message_purifier.dart new file mode 100644 index 000000000..6516e27e6 --- /dev/null +++ b/core/lib/presentation/validator/html_message_purifier.dart @@ -0,0 +1,42 @@ +import 'sane_html_validator.dart' show SaneHtmlValidator; +import 'package:core/core.dart'; + +class HtmlMessagePurifier { + String purifyHtmlMessage( + String html, + { + Set? allowElementIds, + Set? allowClassNames, + Set? allowAttributes + } + ) { + return sanitizeHtml( + html, + allowElementId: (elementId) => allowElementIds != null ? allowElementIds.contains(elementId) : false, + allowClassName: (className) => allowClassNames != null ? allowClassNames.contains(className) : false, + allowAttributes: (attribute) => allowAttributes != null ? allowAttributes.contains(attribute) : false) + .changeStyleBackground() + .removeFontSizeZeroPixel() + .removeMaxHeightZeroPixel() + .removeMaxWidthZeroPixel() + .removeHeightZeroPixel() + .removeWidthZeroPixel(); + } + + String sanitizeHtml( + String htmlString, + { + bool Function(String)? allowElementId, + bool Function(String)? allowClassName, + bool Function(String)? allowAttributes, + Iterable? Function(String)? addLinkRel + } + ) { + return SaneHtmlValidator( + allowElementId: allowElementId, + allowClassName: allowClassName, + allowAttributes: allowAttributes, + addLinkRel: addLinkRel, + ).sanitize(htmlString); + } +} \ No newline at end of file diff --git a/core/lib/presentation/validator/sane_html_validator.dart b/core/lib/presentation/validator/sane_html_validator.dart new file mode 100644 index 000000000..2c4669d3a --- /dev/null +++ b/core/lib/presentation/validator/sane_html_validator.dart @@ -0,0 +1,274 @@ + +import 'package:html/dom.dart'; +import 'package:html/parser.dart' as html_parser; + +final _allowedElements = { + 'H1', + 'H2', + 'H3', + 'H4', + 'H5', + 'H6', + 'H7', + 'H8', + 'BR', + 'B', + 'I', + 'STRONG', + 'EM', + 'A', + 'PRE', + 'CODE', + 'IMG', + 'TT', + 'DIV', + 'INS', + 'DEL', + 'SUP', + 'SUB', + 'P', + 'OL', + 'UL', + 'TABLE', + 'THEAD', + 'TBODY', + 'TFOOT', + 'BLOCKQUOTE', + 'DL', + 'DT', + 'DD', + 'KBD', + 'Q', + 'SAMP', + 'VAR', + 'HR', + 'RUBY', + 'RT', + 'RP', + 'LI', + 'TR', + 'TD', + 'TH', + 'S', + 'STRIKE', + 'SUMMARY', + 'DETAILS', + 'CAPTION', + 'FIGURE', + 'FIGCAPTION', + 'ABBR', + 'BDO', + 'CITE', + 'DFN', + 'MARK', + 'SMALL', + 'SPAN', + 'TIME', + 'WBR', +}; + +final _alwaysAllowedAttributes = { + 'abbr', + 'accept', + 'accept-charset', + 'accesskey', + 'action', + 'align', + 'alt', + 'aria-describedby', + 'aria-hidden', + 'aria-label', + 'aria-labelledby', + 'axis', + 'border', + 'cellpadding', + 'cellspacing', + 'char', + 'charoff', + 'charset', + 'checked', + 'clear', + 'cols', + 'colspan', + 'color', + 'compact', + 'coords', + 'datetime', + 'dir', + 'disabled', + 'enctype', + 'for', + 'frame', + 'headers', + 'height', + 'hreflang', + 'hspace', + 'ismap', + 'label', + 'lang', + 'maxlength', + 'media', + 'method', + 'multiple', + 'name', + 'nohref', + 'noshade', + 'nowrap', + 'open', + 'prompt', + 'readonly', + 'rel', + 'rev', + 'rows', + 'rowspan', + 'rules', + 'scope', + 'selected', + 'shape', + 'size', + 'span', + 'start', + 'summary', + 'tabindex', + 'target', + 'title', + 'type', + 'usemap', + 'valign', + 'value', + 'vspace', + 'width', + 'itemprop', +}; + +bool _alwaysAllowed(String _) => true; + +bool _validLink(String url) { + try { + final uri = Uri.parse(url); + return uri.isScheme('https') || + uri.isScheme('http') || + uri.isScheme('mailto') || + !uri.hasScheme; + } on FormatException { + return false; + } +} + +bool _validUrl(String url) { + try { + final uri = Uri.parse(url); + return uri.isScheme('https') || uri.isScheme('http') || !uri.hasScheme; + } on FormatException { + return false; + } +} + +final _citeAttributeValidator = { + 'cite': _validUrl, +}; + +final _elementAttributeValidators = + >{ + 'A': { + 'href': _validLink, + }, + 'IMG': { + 'src': _alwaysAllowed, + 'longdesc': _validUrl, + }, + 'DIV': { + 'itemscope': _alwaysAllowed, + 'itemtype': _alwaysAllowed, + }, + 'BLOCKQUOTE': _citeAttributeValidator, + 'DEL': _citeAttributeValidator, + 'INS': _citeAttributeValidator, + 'Q': _citeAttributeValidator, + }; + +/// An implementation of [html.NodeValidator] that only allows sane HTML tags +/// and attributes protecting against XSS. +/// +/// Modeled after the [rules employed by Github][1] when sanitizing GFM (Github +/// Flavored Markdown). Notably this excludes CSS styles and other tags that +/// easily interferes with the rest of the page. +/// +/// [1]: https://github.com/jch/html-pipeline/blob/master/lib/html/pipeline/sanitization_filter.rb +class SaneHtmlValidator { + final bool Function(String)? allowElementId; + final bool Function(String)? allowClassName; + final bool Function(String)? allowAttributes; + final Iterable? Function(String)? addLinkRel; + + SaneHtmlValidator({ + required this.allowElementId, + required this.allowClassName, + required this.allowAttributes, + required this.addLinkRel, + }); + + String sanitize(String htmlString) { + final root = html_parser.parseFragment(htmlString); + _sanitize(root); + return root.outerHtml; + } + + void _sanitize(Node node) { + if (node is Element) { + final tagName = node.localName!.toUpperCase(); + if (!_allowedElements.contains(tagName)) { + node.remove(); + return; + } + node.attributes.removeWhere((k, v) { + final attrName = k.toString(); + if (attrName == 'id') { + return allowElementId == null || !allowElementId!(v); + } + if (attrName == 'class') { + if (allowClassName == null) return true; + node.classes.removeWhere((cn) => !allowClassName!(cn)); + return node.classes.isEmpty; + } + + return !_isAttributeAllowed(tagName, attrName, v); + }); + if (tagName == 'A') { + final href = node.attributes['href']; + if (href != null && addLinkRel != null) { + final rels = addLinkRel!(href); + if (rels != null && rels.isNotEmpty) { + node.attributes['rel'] = rels.join(' '); + } + } + } + } + if (node.hasChildNodes()) { + // doing it in reverse order, because we could otherwise skip one, when a + // node is removed... + for (var i = node.nodes.length - 1; i >= 0; i--) { + _sanitize(node.nodes[i]); + } + } + } + + bool _isAttributeAllowed(String tagName, String attrName, String value) { + if (_alwaysAllowedAttributes.contains(attrName)) return true; + + if (allowAttributes != null && allowAttributes!(attrName)) return true; + + // Special validators for special attributes on special tags (href/src/cite) + final attributeValidators = _elementAttributeValidators[tagName]; + if (attributeValidators == null) { + return false; + } + + final validator = attributeValidators[attrName]; + if (validator == null) { + return false; + } + + return validator(value); + } +} \ No newline at end of file diff --git a/core/pubspec.yaml b/core/pubspec.yaml index d49e9aa35..53fd0ca38 100644 --- a/core/pubspec.yaml +++ b/core/pubspec.yaml @@ -42,6 +42,8 @@ dependencies: built_collection: 5.1.0 + html: 0.15.0 + dev_dependencies: flutter_test: sdk: flutter diff --git a/core/test/sane_html_validator_test.dart b/core/test/sane_html_validator_test.dart new file mode 100644 index 000000000..6359094c4 --- /dev/null +++ b/core/test/sane_html_validator_test.dart @@ -0,0 +1,80 @@ +import 'package:core/presentation/validator/sane_html_validator.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + group('sane_html_validator test', () { + + test('sanitize should return clean html not contain style attribute', () async { + + final saneHtmlValidator = SaneHtmlValidator( + allowAttributes: null, + allowClassName: null, + allowElementId: null, + addLinkRel: null + ); + + final cleanHtml = saneHtmlValidator.sanitize('Hello'); + + expect(cleanHtml, isNot(contains('style'))); + expect(cleanHtml, isNot(contains('className'))); + expect(cleanHtml, isNot(contains('idElement'))); + expect(cleanHtml, isNot(contains('href'))); + }); + + test('sanitize should return clean html contain style attribute', () async { + + final saneHtmlValidator = SaneHtmlValidator( + allowAttributes: (attribute) => attribute == 'style', + allowClassName: null, + allowElementId: null, + addLinkRel: null + ); + + final cleanHtml = saneHtmlValidator.sanitize('Hello'); + + expect(cleanHtml, contains('style')); + }); + + test('sanitize should return clean html contain test class name', () async { + + final saneHtmlValidator = SaneHtmlValidator( + allowAttributes: null, + allowClassName: (className) => className == 'test', + allowElementId: null, + addLinkRel: null + ); + + final cleanHtml = saneHtmlValidator.sanitize('Hello elementId == 'test', + addLinkRel: null + ); + + final cleanHtml = saneHtmlValidator.sanitize('Hello href == 'http://example.com/test.jpg' ? ['nofollow'] : null, + ); + + final cleanHtml = saneHtmlValidator.sanitize('Hello'); + + expect(cleanHtml, contains('rel="nofollow"')); + }); + }); +} \ No newline at end of file diff --git a/lib/features/email/presentation/email_view.dart b/lib/features/email/presentation/email_view.dart index c33b83f1a..659287c49 100644 --- a/lib/features/email/presentation/email_view.dart +++ b/lib/features/email/presentation/email_view.dart @@ -18,6 +18,7 @@ class EmailView extends GetWidget { final emailController = Get.find(); final responsiveUtils = Get.find(); final imagePaths = Get.find(); + final htmlMessagePurifier = Get.find(); @override Widget build(BuildContext context) { @@ -192,8 +193,8 @@ class EmailView extends GetWidget { itemCount: messageContents.length, itemBuilder: (context, index) => MessageContentTileBuilder( + htmlMessagePurifier, messageContents[index], - imagePaths, attachmentsInline, emailController.mailboxDashBoardController.sessionCurrent, emailController.mailboxDashBoardController.accountId.value) diff --git a/lib/features/email/presentation/widgets/message_content_tile_builder.dart b/lib/features/email/presentation/widgets/message_content_tile_builder.dart index 765eb2bce..e55fe56a2 100644 --- a/lib/features/email/presentation/widgets/message_content_tile_builder.dart +++ b/lib/features/email/presentation/widgets/message_content_tile_builder.dart @@ -2,27 +2,24 @@ import 'package:core/core.dart'; import 'package:flutter/material.dart'; import 'package:flutter/widgets.dart'; -import 'package:flutter_svg/flutter_svg.dart'; import 'package:flutter_widget_from_html/flutter_widget_from_html.dart'; import 'package:jmap_dart_client/jmap/account_id.dart'; import 'package:jmap_dart_client/jmap/core/session/session.dart'; -import 'package:photo_view/photo_view.dart'; import 'package:tmail_ui_user/features/email/presentation/model/attachment_file.dart'; import 'package:tmail_ui_user/features/email/presentation/model/message_content.dart'; import 'package:tmail_ui_user/features/email/presentation/model/text_format.dart'; -import 'package:sanitize_html/sanitize_html.dart' show sanitizeHtml; class MessageContentTileBuilder { final MessageContent _messageContent; - final ImagePaths _imagePaths; final List _attachmentInlines; final Session? _session; final AccountId? _accountId; + final HtmlMessagePurifier _htmlMessagePurifier; MessageContentTileBuilder( + this._htmlMessagePurifier, this._messageContent, - this._imagePaths, this._attachmentInlines, this._session, this._accountId, @@ -39,10 +36,9 @@ class MessageContentTileBuilder { child: _messageContent.textFormat == TextFormat.PLAIN ? Text('${_messageContent.content}', style: TextStyle(fontSize: 12, color: AppColor.mailboxTextColor)) : HtmlWidget( - cleanHtmlMessage(_messageContent.content), + getHtmlMessageText(), textStyle: TextStyle(fontSize: 12, color: AppColor.mailboxTextColor), buildAsync: true, - factoryBuilder: () => _PopupPhotoViewWidgetFactory(_imagePaths), buildAsyncBuilder: (_, snapshot) => snapshot.data ?? SizedBox( height: 30, width: 30, @@ -57,47 +53,11 @@ class MessageContentTileBuilder { final message = (_attachmentInlines.isNotEmpty && _session != null && _accountId != null && _messageContent.hasImageInlineWithCid()) ? '${_messageContent.getContentHasInlineAttachment(_session!, _accountId!, _attachmentInlines)}' : '${_messageContent.content}'; - return message; - } - String cleanHtmlMessage(String message) => hasTagTable(getHtmlMessageText()) ? sanitizeHtml(getHtmlMessageText()) : message; + final trustAsHtml = _htmlMessagePurifier.purifyHtmlMessage( + message, + allowAttributes: {'style', 'input', 'form'}); - bool hasTagTable(String html) => html.toLowerCase().contains(''); -} - -class _PopupPhotoViewWidgetFactory extends WidgetFactory { - - final ImagePaths _imagePaths; - - _PopupPhotoViewWidgetFactory(this._imagePaths); - - @override - Widget? buildImageWidget(BuildMetadata meta, {String? semanticLabel, required String url}) { - final built = super.buildImageWidget(meta, semanticLabel: semanticLabel, url: url); - - if (built is Image) { - return Builder( - builder: (context) => GestureDetector( - onTap: () => Navigator.of(context).push(MaterialPageRoute( - builder: (_) => Scaffold( - body: Stack( - children: [ - PhotoView( - heroAttributes: PhotoViewHeroAttributes(tag: url), - imageProvider: built.image), - Padding( - padding: EdgeInsets.only(left: 16, top: 35), - child: IconButton( - onPressed: () => Navigator.of(context).pop(), - icon: SvgPicture.asset(_imagePaths.icCloseMailbox, width: 30, height: 30, fit: BoxFit.fill))) - ], - ), - ))), - child: Hero(tag: url, child: built), - ), - ); - } - - return built; + return trustAsHtml; } } \ No newline at end of file diff --git a/lib/main/bindings/core/core_bindings.dart b/lib/main/bindings/core/core_bindings.dart index b02f95ff5..02bdf5868 100644 --- a/lib/main/bindings/core/core_bindings.dart +++ b/lib/main/bindings/core/core_bindings.dart @@ -10,6 +10,7 @@ class CoreBindings extends Bindings { _bindingAppImagePaths(); _bindingResponsiveManager(); _bindingKeyboardManager(); + _bindingValiadtor(); } void _bindingAppImagePaths() { @@ -27,4 +28,8 @@ class CoreBindings extends Bindings { void _bindingKeyboardManager() { Get.put(KeyboardUtils()); } + + void _bindingValiadtor() { + Get.put(HtmlMessagePurifier()); + } } \ No newline at end of file diff --git a/pubspec.yaml b/pubspec.yaml index 6ee1439d8..1abe5f02b 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -89,11 +89,6 @@ dependencies: # flutter_widget_from_html flutter_widget_from_html: 0.6.2 - # photo_view - photo_view: 0.12.0 - - sanitize_html: 2.0.0 - dev_dependencies: flutter_test: sdk: flutter