Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store XSS #1212

Closed
3as0n opened this issue Jun 19, 2020 · 10 comments
Closed

Store XSS #1212

3as0n opened this issue Jun 19, 2020 · 10 comments

Comments

@3as0n
Copy link

3as0n commented Jun 19, 2020

Describe your problem

https://github.com/bludit/bludit/blob/master/bl-kernel/ajax/logo-upload.php

Logo upload only determines the suffix, but not the content, which causes XSS
and the user can inject any javascript and html code in the page

payload
image

image
image

@galaktipus
Copy link

galaktipus commented Jun 25, 2020

Any known fix for issue yet? Seems like it was assigned to CVE-2020-15006

@3as0n
Copy link
Author

3as0n commented Jun 25, 2020 via email

@dignajar
Copy link
Member

I will check, is not a problem for Bludit because the users are trusted by the Administrator, I mean Bludit doesn't have user registration.

Feel free to provide a fix for it.

Regards

@3as0n
Copy link
Author

3as0n commented Jun 26, 2020 via email

@ghost
Copy link

ghost commented Jun 26, 2020

Vector Images are really important (especially for responsive design), but many Content Management Systems avoid them because detecting and filtering XSS is a horrible job.

However, losing SVG Support in Bludit shouldn't be the way to go. So how about the following Validation function (can be part of the Valid helper class). It's may not perfect now, but it works on the most common XSS attacks and I also checked the used Regular Expression against MDN's SVG Attribute's list to avoid false positives.

<?php

class Valid
{
    static public function svg($content, $requireXmlTag = false, $requireDoctypeTag = false) {
        if($requireXmlTag && stripos($content, "<?xml") !== 0) {
            return false;
        }
        if($requireDoctypeTag && stripos($content, "<!DOCTYPE") === false) {
            return false;
        }

        // Validate XML Format
        try {
            $xml = @new SimpleXMLElement($content);
        } catch(Exception $e) {
            $xml = false;
        }
        if(!$xml) {
            return false;
        }

        // Check Basic XSS
        if(!!preg_match("/(<(\/)?script|on[a-z]+=|(java)?script:)/i", $content)) {
            return false;
        }
        return true;
    }
}

~ Sam.

@ghost
Copy link

ghost commented Jun 26, 2020

We can also add the following regular expression to avoid URLs except the w3.org standards too, which may leads to false positives if someone adds a URL as comment. This is also not fully tested yet.

(http.)?(:\/\/)?(www\.)(?!w3\.org)

@working-name
Copy link

@SamBrishes That regex is a good start but it gets complicated fast: https://owasp.org/www-community/xss-filter-evasion-cheatsheet

Especially when you can have whitespace in the word 'javascript:'. Let me know if you're serious about using regex for this, I'd be happy to make one that catches the known methods people use to bypass xss checks.

@3as0n
Copy link
Author

3as0n commented Jun 27, 2020 via email

@ghost
Copy link

ghost commented Jun 27, 2020

Especially when you can have whitespace in the word 'javascript:'. Let me know if you're serious about using regex for this, I'd be happy to make one that catches the known methods people use to bypass xss checks.

Feel free to bypass the regex, but keep in mind, that it still need to be a valid XML file according to SimpleXMLElement.

Of course, we can also use an (adapted) version of the Ulf Harnhammar's Kses Library and check each single tag and attribute available in a SVG image only, but I don't think that this is really necessary.

Anyway, Bludit should also just allow admins to upload SVGs to restrict the responsibility and I guess it is also important to check the MIME type of images too, next to the file extension. (For example as I'm using it on my media plugin).

dignajar added a commit that referenced this issue Jun 29, 2020
@dignajar
Copy link
Member

Hello, with the new version of Bludit v4.0 rc1, I would like to close the old Github issues. If you feel that your issue is not resolved in the latest version, create a new ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants