Fixed Inconsistent (and potentially dangerous) output for URL bbcode due to censor and unfurl

K

Kirby

Well-known member
Steps to reproduce
  1. Add the following censored words with replacement
    Word or phraseReplacement (optional)
    replaceword
    censorwordreplaceword
    https://javascriptjavascript
  2. Add posts with the following BB code
    1. [url unfurl="true"]https://censorword.does.not.exist[/url]
    2. [url]https://censorword.does.not.exist[/url]
    3. [url unfurl="true"]about://censorword.does.not.exist[/url]
    4. https://javascript:alert('called!');
  3. View rendered HTML with unfurl enabled
  4. View rendered HTML with unfurl disabled

Expected Result
In cases 1) and 2) the same censored link is shown in step 3) and 4)
In cases 3) and 4) the URL in shown as plaintext in step 3) and 4)

Actual Result
In case 1) the output differs between step 3) and 4)
In case 3) the output is invisibble in step 4)
In case 4) the output as a clickable link, effectively allowing the user to inject JavaScript.


Replacing method XF\BbCodeRenderer\Html::renderTagUrl with smth. like the following code (+ additional changes to support new option noCensor) seems to fix this for me:
PHP: 
public function shouldUnfurl(string $url, $option, array $options): bool
{
    if (
        is_array($option)
        && isset($option['unfurl'])
        && $option['unfurl'] === 'true'
        && !empty($options['allowUnfurl'])
        )
    {
        return true;
    }

    return false;
}

public function renderTagUrl(array $children, $option, array $tag, array $options)
{
    $unfurl = false;

    $options['noCensor'] = true;

    $text = '';

    if ($option !== null && !is_array($option))
    {
        $options['lightbox'] = false;

        $url = $option;
        $text = $this->renderSubTree($children, $options);
    }
    else
    {
        $unfurl = true;
        $url = $this->renderSubTreePlain($children);
    }

    $url = $this->formatter->censorText($url);

    if ($text === '')
    {
        $text = $this->prepareTextFromUrlExtended($url, $options);
    }

    $url = $this->getValidUrl($url);

    if (!$url)
    {
        return $text;
    }

    if ($unfurl && $this->shouldUnfurl($url, $option, $options))
    {
        return $this->getRenderedUnfurl($url, $options, $text);
    }

    return $this->getRenderedLink($text, $url, $options);
}
 
Thank you for reporting this issue, it has now been resolved. We are aiming to include any changes that have been made in a future XF release (2.2.6).

Change log:
Do not attempt to link URLs or email addresses that contain censored words.
Click to expand...
There may be a delay before changes are rolled out to the XenForo Community.
 
Kirby said:
In case 3) the output is invisibble in step 4)
Click to expand...
This bit was also changed so it outputs the "text" which is out the non-unfurl situation works. (Though it's wrapped in a div as the unfurl version is considered a block element.)

Beyond that, if running the URL (or email address) through the censorship process changes it, we'll no longer attempt to link it. We weren't totally consistent with this, as images (and media) already behaved like this. In the vast majority of cases, this will be a better/more expected behavior -- links that have "****" in the URL aren't overly helpful anyway (and they won't work). There is one situation that this could affect, though it's really not the intention of the censoring options: rewriting content to change something like olddomain.com to newdomain.com. If that comes up, we'd probably recommend using content replacement tools, as you'd fix things like images as well.
 
Hmm, I am not sure if I should report the new implementation as a new bug or reopen this one?

With the changes made in 2.2.6 it is now possible to post smth. like
[url="https://censorword I am going to hide this next, it will only be shown if somebody does quote or edit this post"]Just some normal text[/url] to effectively "hide text", the output for a viewer would be just Just some normal text.

This is not good as it could easily be missed by moderators.
 
Last edited:
Back
Top Bottom