Skip to content

Conversation

@dab246
Copy link
Member

@dab246 dab246 commented Nov 27, 2025

Issue

Email content does not display because the HTML sanitization process removes the <form> tag and all its nested elements. This causes parts of the email body to disappear when rendering.

Root cause

The email content disappeared because the HTML sanitization pipeline removed the <form> tag, which caused all elements nested inside the form to be stripped during processing.

Solution

  • Allow the <form> tag to be preserved by extending the sanitization allowlist.
  • Introduce and use the environment variable EMAIL_HTML_TAGS_TO_PRESERVE so deployments can append custom tags (including form) without modifying code.

Resolved

@github-actions
Copy link

This PR has been deployed to https://linagora.github.io/tmail-flutter/4184.

@hoangdat
Copy link
Member

  1. Form Tags Are Dangerous by Design
  The original sanitize_html library explicitly strips <form> elements for security reasons. Your fork now allows them, which creates
  phishing and form submission attack risks.

  2. Missing Dangerous Attribute Tests
  The only test checks href attribute (which isn't even valid on forms). Missing tests for:
  - action - Where form submits data (could be attacker URL)
  - formaction - Overrides action on inputs/buttons
  - method, formmethod - HTTP methods
  - target, formtarget - Where response opens
  - enctype, formenctype - Encoding type

  Need to verify these are being stripped by the library.

  3. No Validation of Environment Variable
  EMAIL_HTML_TAGS_TO_PRESERVE accepts any tags without validation. An operator could accidentally add script, iframe, object:

  EMAIL_HTML_TAGS_TO_PRESERVE=script,iframe,form  # No validation stops this!

  4. Performance Issue
  loadPreservedHtmlTags() is called every time process() runs
  (core/lib/presentation/utils/html_transformer/text/standardize_html_sanitizing_transformers.dart:39), re-parsing the env variable each
  time.

  5. Case Sensitivity
  form and FORM are treated as different tags (see test at core/test/utils/html_sanitize_config_test.dart:117-121), but HTML is
  case-insensitive.

@hoangdat
Copy link
Member

Recommended Security Tests

  // Test form-specific dangerous attributes
  test('SHOULD remove action attribute from form', () {
    const input = '<form action="https://attacker.com/steal"></form>';
    expect(transformer.process(input, htmlEscape), equals('<form></form>'));
  });

  test('SHOULD remove formaction from input/button elements', () {
    const input = '<form><input formaction="https://evil.com"></form>';
    // Verify formaction is stripped
  });

  // Test environment variable validation
  test('SHOULD reject dangerous tags from env variable', () {
    dotenv.testLoad(mergeWith: {
      'EMAIL_HTML_TAGS_TO_PRESERVE': 'script,iframe',
    });
    const input = '<script>alert(1)</script>';
    expect(transformer.process(input, htmlEscape), equals(''));
  });

  // Test combined XSS vectors
  test('SHOULD sanitize form with multiple dangerous attributes', () {
    const input = '<form action="javascript:alert(1)" method="POST" onsubmit="alert(1)"></form>';
    expect(transformer.process(input, htmlEscape), equals('<form></form>'));
  });

@hoangdat
Copy link
Member

Recommended Fixes

  1. Add Tag Validation (core/lib/presentation/utils/html_transformer/html_sanitize_config.dart:7-25)
  static const _blockedTags = ['script', 'iframe', 'object', 'embed', 'input', 'button', 'textarea'];

  static List<String> loadPreservedHtmlTags() {
    final tags = _parseFromEnv();
    return tags.where((t) => !_blockedTags.contains(t.toLowerCase())).toList();
  }

  3. Normalize Case (same file)
  .map((e) => e.trim().toLowerCase())  // Add .toLowerCase()

@dab246
Copy link
Member Author

dab246 commented Dec 2, 2025

  1. Form Tags Are Dangerous by Design
  The original sanitize_html library explicitly strips <form> elements for security reasons. Your fork now allows them, which creates
  phishing and form submission attack risks.

  2. Missing Dangerous Attribute Tests
  The only test checks href attribute (which isn't even valid on forms). Missing tests for:
  - action - Where form submits data (could be attacker URL)
  - formaction - Overrides action on inputs/buttons
  - method, formmethod - HTTP methods
  - target, formtarget - Where response opens
  - enctype, formenctype - Encoding type

  Need to verify these are being stripped by the library.

  3. No Validation of Environment Variable
  EMAIL_HTML_TAGS_TO_PRESERVE accepts any tags without validation. An operator could accidentally add script, iframe, object:

  EMAIL_HTML_TAGS_TO_PRESERVE=script,iframe,form  # No validation stops this!

  4. Performance Issue
  loadPreservedHtmlTags() is called every time process() runs
  (core/lib/presentation/utils/html_transformer/text/standardize_html_sanitizing_transformers.dart:39), re-parsing the env variable each
  time.

  5. Case Sensitivity
  form and FORM are treated as different tags (see test at core/test/utils/html_sanitize_config_test.dart:117-121), but HTML is
  case-insensitive.

Your AI analysis is quite correct, but not enough, we have too many attributes that are easily affected from the outside. So we need to normalize the library, instead of keeping the tags we will analyze deeper to remove that tag and go to its child tags.

@dab246
Copy link
Member Author

dab246 commented Dec 3, 2025

Solution 2 is implemented here #4188

@hoangdat hoangdat closed this Dec 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants