HackerOne users: Testing against this community violates our program's Terms of Service and will result in your bounty being denied.

Title with trailing dots (embed version) not handled correctly...

edited January 2012 in Feedback

On an embed (with WordPress) Vanilla forum, a title with trailing dots like "dots..." will be converted to url like "http://example.com/discussions/dots...", which cannot be resolved correctly.

Please do handle all the special characters in the title (url) carefully since this is may lead to an injection or XSS attack easily...

Tagged:

Best Answer

  • x00x00 MVP
    Answer ✓

    Btw what your suggesting could be done as a plugin. There is a similar clean urls/slug plugin for wordpress.

    grep is your friend.

Answers

  • 422422 Developer MVP

    How do you get xss attack from a url that your forum poster posts with dots in...

    There was an error rendering this rich post.

  • edited January 2012

    @Developer

    (I know only a little php, please forgive me if I make a mistake.)

    I dag into the code a bit, and found the cause of this bug:
    In function Gdn_Format::Url (line 1209 in library\core\class.format.php) you wrote

    preg_replace('`([^\PP.\-_])`u', '', $Mixed)

    this regex excludes _ . - from being replaced, which is reasonable for a full url like www.example-abc.com , but not for a title like "title contains dots...".

    I'll submit this bug at GitHub soon.

    @422
    It's reasonable to think that some other special characters are also not converted correctly. A poster may be able to inject a piece of html into the title.

  • x00x00 MVP
    edited January 2012

    That function merely formats the url, it is not what formats the title.

    Title is linked to the record anyway.

    grep is your friend.

  • edited January 2012

    x00 said:
    That function merely formats the url, it is not what formats the title.

    Title is linked to the record anyway.

    Yes, it is. All urls are generated from titles dynamically. For example, the url of this page is generated by the following line:

    $Discussion->Url = Url('/discussion/'.$Discussion->DiscussionID.'/'.Gdn_Format::Url($Discussion->Name), TRUE);

    $Discussion->Name is the title ("Title with trailing dots (embed version) not handled correctly...").

    This forum must use a modified version of Vanilla. I just tried a few other forums, all have this problem. You can try it on your own.

  • I've seen this happen in my own forum. It would be nice if periods were stripped in slugs.

  • ChanningD you are missing the point. That generates the url. It is one way. it doesn't allow code to be inserted in the title. Because the title is not derived from the url, it is derived from the record, which is already sanitised. In fact the title bit is superficial, all you really need is the ID. It is called a 'slug'.

    A little knowledge, means you need read up some more.

    grep is your friend.

  • there is no security issue there.

    grep is your friend.

  • edited January 2012

    @x00

    ChanningD said:
    Please do handle all the special characters in the title (url) carefully since this is may lead to an injection or XSS attack easily...

    If Gdn_Format::Url fails, why should I trust Gdn_Format::Title (may be called something else) or Gdn_Format::Whatever? That's what I'm trying to say.

  • edited January 2012

    x00 said:
    ChanningD you are missing the point. That generates the url. It is one way. it doesn't allow code to be inserted in the title. Because the title is not derived from the url, it is derived from the record, which is already sanitised. In fact the title bit is superficial, all you really need is the ID. It is called a 'slug'.

    I know this. What I really mean is Gdn_Format::Url formats the title to (part of) the url. Sorry if my English is confusing.

    A little knowledge, means you need read up some more.

    Sure. I'm still learning php ;)

    x00 said:
    there is no security issue there.

    Maybe. But don't you think it's frustrating if any user can generate urls like www.a.com/discussion/123/harmless.php (if the title is "harmless.php") or www.a.com/discussion/123/.... ?

  • x00x00 MVP
    edited January 2012

    Well those urls still have to be resolved, it is the same with any site.

    Garden/Vanilla explicitly denies scripts from being accessed directly.

    I would still issue a bug report about the trailing dots, it is not a difficult issue to resolve.

    temporary fix would be to use rtrim(trim($Discussion->Name),'.')

    grep is your friend.

  • edited January 2012

    The problem is a lot of servers cannot resolve them correctly. For example on an nginx server with default settings, the url www.a.com/discussion/123/harmless.php will be send to fastcgi as php directly instead of being reparsed to index.php?p=/discussion/123/harmless.php

    I do think no dots should be allowed in slugs at all.

  • what I mean is in the file is

    <?php if (!defined('APPLICATION')) exit();
    

    grep is your friend.

  • x00x00 MVP
    edited January 2012

    That is a decision for the dev team

    at the end of the day it is still your responsibility to configure the server

    nginx works well with vanilla, but is still not the officially supported server, becuase ti does require a lot of configuration.

    grep is your friend.

  • x00x00 MVP
    Answer ✓

    Btw what your suggesting could be done as a plugin. There is a similar clean urls/slug plugin for wordpress.

    grep is your friend.

Sign In or Register to comment.