Users running a non-download version of Vanilla (pulled from github), on branch release/2019.016 or master from the last 2 weeks should upgrade to release/2019.017 or latest master for security reasons. Downloaded official open sources releases are not affected.

External links entered without https are changed to unsafe

edited February 3 in Vanilla 3.x Help

Vanilla version: 3.3

If I use the rich editor to link to a site where it says "Paste or input URL" and include https in URL, all is fine.

If I omit the protocol or link to a http site, vanilla changes the URL protocol to unsafe and therefore doesn't work.

OK

Not OK

My site is running perfectly fine under SSL but I don't know how/why external links are getting changed like this? Why would someone visiting a http external site from my forum be considered unsafe?

Same thing seems to be happening here. Why?

Comments

  • R_JR_J Cheerleader & Troubleshooter Munich Moderator

    Can you please create an issue on GitHub for that? I can't imagine that this is intended behaviour.


  • I am sure there was some reasoning behind it, although the conclusion is a bit unexpected ... maybe a plugin can change it..

    In vanilla-3.3\library\core\class.format.php

    const SAFE_PROTOCOLS = [
        "http",
        "https",
        "tel",
        "mailto",
    ];
    
    
    /**
     * Sanitize a URL to ensure that it matches a whitelist of approved url schemes.
     * If the url does not match one of these schemes, prepend `unsafe:` before it.
     *
     * Allowed protocols
     * - "http:",
     * - "https:",
     * - "tel:",
     * - "mailto:",
     *
     * @param string $url The url to sanitize.
     *
     * @return string
     */
    public static function sanitizeUrl(string $url): string {
        $protocol = parse_url($url, PHP_URL_SCHEME) ?: "";
        $isSafe = in_array($protocol, self::SAFE_PROTOCOLS, true);
    
        if ($isSafe) {
            return $url;
        } else {
            return "unsafe:".$url;
        }
    }
    
  • edited February 5

    Ok so the main issue here then is when you don't enter a protocol at all, it doesn't get rewritten as http or https, but unsafe.

  • R_JR_J Cheerleader & Troubleshooter Munich Moderator

    @donshakespeare I really expect that this must be considered a UX bug. Just yesterday I have produced such an unclickable link here.

    I know that a valid link technically requires the protocol, but as a user my expectation is that what I enter into the address line of my browser is the link and that I can enter that into that "Link" popup and everything is fine.


    donshakespearerbrahmson
  • edited February 5

    Yes it is very unpleasant and unexpected indeed. I suggest that if the reasons for this implementation exist and bars the idea of bug, then at least we need a way to easily configure the accepted protocols.

    I often don't know the protocol of the site's address I am typing from memory. I expect this behaviour to default to blank protocol and let the browser figure it out.

    Seems Angular2 is also notorious for this:

    https://stackoverflow.com/questions/15606751/angularjs-changes-urls-to-unsafe-in-extension-page

    https://serviceportal.io/avoid-unsafe-prefix-links/

  • rbrahmsonrbrahmson "You may say I'm a dreamer / But I'm not the only one" NY ✭✭✭

    I'm not sure what's the downside to prepend http or https (choice could be in config) in any protocol lacking input that's is expected to have one. Therefore I tend to concur that short of clarification by the Vanilla team this is a bug.

    When I write plugins I am using the above assumption (e.g. FeedDiscussionPlus).

  • charrondevcharrondev Developer Lead (PHP, JS) Montreal Vanilla Staff

    Yeah, we could probably be a bit smarter about it. I'm not opposed to changing the behaviour to switch to append https:// instead of unsafe.

    I think we also have an allowed safe protocol group that's configurable, and just doesn't happen to be used here. I think this ended up being put in as a quick security patch, and didn't get as much thought as it probably should have.

    rbrahmson
Sign In or Register to comment.