Mautic: Trackable redirect incorrectly parses the end of the URL in a plain text version

Created on 11 Aug 2018  路  4Comments  路  Source: mautic/mautic

Please DO NOT report security vulnerabilities here. Send them to [email protected] instead.

What type of report is this:

| Q | A
| ---| ---
| Bug report? | Y
| Feature request? |
| Enhancement? |

Description:

This issue was introduced after updating to 2.14.0 (and probably after merging https://github.com/mautic/mautic/pull/6260).
After clicking on "Auto Generate" to create a plaintext version of email the links are shown in square brackets. After sending this email, the right square bracket is not removed from the URL, thereby creating an incorrect page redirect (you can see it in the database in table page_redirects, column url).

If a bug:

| Q | A
| --- | ---
| Mautic version | 2.14.0
| PHP version | 7.1.18

Steps to reproduce:

  1. Create a new segment email
  2. In the builder, add any URL that does not navigate to the homepage, and after the domain name there is at least one slash (https://example.org/page).
  3. Click the "Auto Generate" button to create a plaintext version of the email.
  4. Send this email.

Or use this plain text sample

Sample plain text email

This email contains URL with the square brackets [https://example.org/with/square/brackets]
also the square brackets with a slash and a comma [https://example.org/square/brackets/with/slash/and/comma/],
or parentheses (https://example.org/with/parentheses)
even with just a comma: https://example.org/with/comma,
or with a dot: https://example.org/with/dot.

Expected result

Actual result

Personally for myself, I fixed it this way

In this regex
https://github.com/mautic/mautic/blob/0927aef5b0e54b5036088f11e42642b99731cf24/app/bundles/CoreBundle/Helper/UrlHelper.php#L169
Changed this
... ?(?:/[^\s]*)?_ius';
To this
... ?(?:/[^\s\]\)\.,;]*)?_ius';

Log errors:

_Please check for related errors in the latest log file in [mautic root]/app/log/ and/or the web server's logs and post them here. Be sure to remove sensitive information if applicable._

bug

All 4 comments

@alanhartless what do you think of this please?

My thoughts are to have someone PR the change then have people test it. Add unit tests with different plain text pay loads and the different URLs to ensure they pass in any scenario we can think of.

Thanks @alanhartless let me check if we can handle that.

Hi @StudioMaX & @alanhartless feel free to test and review #6474 :)

Was this page helpful?
0 / 5 - 0 ratings