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

ChanningD

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...

x00

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

422

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

ChanningD

@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.

x00

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

Title is linked to the record anyway.

ChanningD

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.

hofr

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

x00

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.

x00

there is no security issue there.

ChanningD

@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.

ChanningD

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/.... ?

x00

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),'.')

ChanningD

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.

x00

what I mean is in the file is

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

x00

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.