Merge pull request #495 from aidantwoods/anti-xss

Prevent various XSS attacks [rebase and update of #276]
erusev · Feb 28, 2018 · 6678d59 · 6678d59
2 parents c999a4b + 67c3efb
commit 6678d59
Show file tree

Hide file tree

Showing 8 changed files with 195 additions and 17 deletions.
diff --git a/Parsedown.php b/Parsedown.php
@@ -75,6 +75,32 @@ function setUrlsLinked($urlsLinked)
 
     protected $urlsLinked = true;
 
+    function setSafeMode($safeMode)
+    {
+        $this->safeMode = (bool) $safeMode;
+
+        return $this;
+    }
+
+    protected $safeMode;
+
+    protected $safeLinksWhitelist = array(
+        'http://',
+        'https://',
+        'ftp://',
+        'ftps://',
+        'mailto:',
+        'data:image/png;base64,',
+        'data:image/gif;base64,',
+        'data:image/jpeg;base64,',
+        'irc:',
+        'ircs:',
+        'git:',
+        'ssh:',
+        'news:',
+        'steam:',
+    );
+
     #
     # Lines
     #
@@ -342,8 +368,6 @@ protected function blockCodeComplete($Block)
     {
         $text = $Block['element']['text']['text'];
 
-        $text = htmlspecialchars($text, ENT_NOQUOTES, 'UTF-8');
-
         $Block['element']['text']['text'] = $text;
 
         return $Block;
@@ -354,7 +378,7 @@ protected function blockCodeComplete($Block)
 
     protected function blockComment($Line)
     {
-        if ($this->markupEscaped)
+        if ($this->markupEscaped or $this->safeMode)
         {
             return;
         }
@@ -457,8 +481,6 @@ protected function blockFencedCodeComplete($Block)
     {
         $text = $Block['element']['text']['text'];
 
-        $text = htmlspecialchars($text, ENT_NOQUOTES, 'UTF-8');
-
         $Block['element']['text']['text'] = $text;
 
         return $Block;
@@ -515,10 +537,10 @@ protected function blockList($Line)
                 ),
             );
 
-            if($name === 'ol') 
+            if($name === 'ol')
             {
                 $listStart = stristr($matches[0], '.', true);
-                
+
                 if($listStart !== '1')
                 {
                     $Block['element']['attributes'] = array('start' => $listStart);
@@ -678,7 +700,7 @@ protected function blockSetextHeader($Line, array $Block = null)
 
     protected function blockMarkup($Line)
     {
-        if ($this->markupEscaped)
+        if ($this->markupEscaped or $this->safeMode)
         {
             return;
         }
@@ -1074,7 +1096,6 @@ protected function inlineCode($Excerpt)
         if (preg_match('/^('.$marker.'+)[ ]*(.+?)[ ]*(?<!'.$marker.')\1(?!'.$marker.')/s', $Excerpt['text'], $matches))
         {
             $text = $matches[2];
-            $text = htmlspecialchars($text, ENT_NOQUOTES, 'UTF-8');
             $text = preg_replace("/[ ]*\n/", ' ', $text);
 
             return array(
@@ -1253,8 +1274,6 @@ protected function inlineLink($Excerpt)
             $Element['attributes']['title'] = $Definition['title'];
         }
 
-        $Element['attributes']['href'] = str_replace(array('&', '<'), array('&amp;', '&lt;'), $Element['attributes']['href']);
-
         return array(
             'extent' => $extent,
             'element' => $Element,
@@ -1263,7 +1282,7 @@ protected function inlineLink($Excerpt)
 
     protected function inlineMarkup($Excerpt)
     {
-        if ($this->markupEscaped or strpos($Excerpt['text'], '>') === false)
+        if ($this->markupEscaped or $this->safeMode or strpos($Excerpt['text'], '>') === false)
         {
             return;
         }
@@ -1343,14 +1362,16 @@ protected function inlineUrl($Excerpt)
 
         if (preg_match('/\bhttps?:[\/]{2}[^\s<]+\b\/*/ui', $Excerpt['context'], $matches, PREG_OFFSET_CAPTURE))
         {
+            $url = $matches[0][0];
+
             $Inline = array(
                 'extent' => strlen($matches[0][0]),
                 'position' => $matches[0][1],
                 'element' => array(
                     'name' => 'a',
-                    'text' => $matches[0][0],
+                    'text' => $url,
                     'attributes' => array(
-                        'href' => $matches[0][0],
+                        'href' => $url,
                     ),
                 ),
             );
@@ -1363,7 +1384,7 @@ protected function inlineUrlTag($Excerpt)
     {
         if (strpos($Excerpt['text'], '>') !== false and preg_match('/^<(\w+:\/{2}[^ >]+)>/i', $Excerpt['text'], $matches))
         {
-            $url = str_replace(array('&', '<'), array('&amp;', '&lt;'), $matches[1]);
+            $url = $matches[1];
 
             return array(
                 'extent' => strlen($matches[0]),
@@ -1401,6 +1422,11 @@ protected function unmarkedText($text)
 
     protected function element(array $Element)
     {
+        if ($this->safeMode)
+        {
+            $Element = $this->sanitiseElement($Element);
+        }
+
         $markup = '<'.$Element['name'];
 
         if (isset($Element['attributes']))
@@ -1412,7 +1438,7 @@ protected function element(array $Element)
                     continue;
                 }
 
-                $markup .= ' '.$name.'="'.$value.'"';
+                $markup .= ' '.$name.'="'.self::escape($value).'"';
             }
         }
 
@@ -1426,7 +1452,7 @@ protected function element(array $Element)
             }
             else
             {
-                $markup .= $Element['text'];
+                $markup .= self::escape($Element['text'], true);
             }
 
             $markup .= '</'.$Element['name'].'>';
@@ -1485,10 +1511,77 @@ function parse($text)
         return $markup;
     }
 
+    protected function sanitiseElement(array $Element)
+    {
+        static $goodAttribute = '/^[a-zA-Z0-9][a-zA-Z0-9-_]*+$/';
+        static $safeUrlNameToAtt  = array(
+            'a'   => 'href',
+            'img' => 'src',
+        );
+
+        if (isset($safeUrlNameToAtt[$Element['name']]))
+        {
+            $Element = $this->filterUnsafeUrlInAttribute($Element, $safeUrlNameToAtt[$Element['name']]);
+        }
+
+        if ( ! empty($Element['attributes']))
+        {
+            foreach ($Element['attributes'] as $att => $val)
+            {
+                # filter out badly parsed attribute
+                if ( ! preg_match($goodAttribute, $att))
+                {
+                    unset($Element['attributes'][$att]);
+                }
+                # dump onevent attribute
+                elseif (self::striAtStart($att, 'on'))
+                {
+                    unset($Element['attributes'][$att]);
+                }
+            }
+        }
+
+        return $Element;
+    }
+
+    protected function filterUnsafeUrlInAttribute(array $Element, $attribute)
+    {
+        foreach ($this->safeLinksWhitelist as $scheme)
+        {
+            if (self::striAtStart($Element['attributes'][$attribute], $scheme))
+            {
+                return $Element;
+            }
+        }
+
+        $Element['attributes'][$attribute] = str_replace(':', '%3A', $Element['attributes'][$attribute]);
+
+        return $Element;
+    }
+
     #
     # Static Methods
     #
 
+    protected static function escape($text, $allowQuotes = false)
+    {
+        return htmlspecialchars($text, $allowQuotes ? ENT_NOQUOTES : ENT_QUOTES, 'UTF-8');
+    }
+
+    protected static function striAtStart($string, $needle)
+    {
+        $len = strlen($needle);
+
+        if ($len > strlen($string))
+        {
+            return false;
+        }
+        else
+        {
+            return strtolower(substr($string, 0, $len)) === strtolower($needle);
+        }
+    }
+
     static function instance($name = 'default')
     {
         if (isset(self::$instances[$name]))

diff --git a/test/ParsedownTest.php b/test/ParsedownTest.php
@@ -48,6 +48,8 @@ function test_($test, $dir)
         $expectedMarkup = str_replace("\r\n", "\n", $expectedMarkup);
         $expectedMarkup = str_replace("\r", "\n", $expectedMarkup);
 
+        $this->Parsedown->setSafeMode(substr($test, 0, 3) === 'xss');
+
         $actualMarkup = $this->Parsedown->text($markdown);
 
         $this->assertEquals($expectedMarkup, $actualMarkup);

diff --git a/test/data/xss_attribute_encoding.html b/test/data/xss_attribute_encoding.html
@@ -0,0 +1,6 @@
+<p><a href="https://www.example.com&quot;">xss</a></p>
+<p><img src="https://www.example.com&quot;" alt="xss" /></p>
+<p><a href="https://www.example.com&#039;">xss</a></p>
+<p><img src="https://www.example.com&#039;" alt="xss" /></p>
+<p><img src="https://www.example.com" alt="xss&quot;" /></p>
+<p><img src="https://www.example.com" alt="xss&#039;" /></p>
diff --git a/test/data/xss_attribute_encoding.md b/test/data/xss_attribute_encoding.md
@@ -0,0 +1,11 @@
+[xss](https://www.example.com")
+
+![xss](https://www.example.com")
+
+[xss](https://www.example.com')
+
+![xss](https://www.example.com')
+
+![xss"](https://www.example.com)
+
+![xss'](https://www.example.com)
diff --git a/test/data/xss_bad_url.html b/test/data/xss_bad_url.html
@@ -0,0 +1,16 @@
+<p><a href="javascript%3Aalert(1)">xss</a></p>
+<p><a href="javascript%3Aalert(1)">xss</a></p>
+<p><a href="javascript%3A//alert(1)">xss</a></p>
+<p><a href="javascript&amp;colon;alert(1)">xss</a></p>
+<p><img src="javascript%3Aalert(1)" alt="xss" /></p>
+<p><img src="javascript%3Aalert(1)" alt="xss" /></p>
+<p><img src="javascript%3A//alert(1)" alt="xss" /></p>
+<p><img src="javascript&amp;colon;alert(1)" alt="xss" /></p>
+<p><a href="data%3Atext/html;base64,PHNjcmlwdD5hbGVydCgxKTwvc2NyaXB0Pg==">xss</a></p>
+<p><a href="data%3Atext/html;base64,PHNjcmlwdD5hbGVydCgxKTwvc2NyaXB0Pg==">xss</a></p>
+<p><a href="data%3A//text/html;base64,PHNjcmlwdD5hbGVydCgxKTwvc2NyaXB0Pg==">xss</a></p>
+<p><a href="data&amp;colon;text/html;base64,PHNjcmlwdD5hbGVydCgxKTwvc2NyaXB0Pg==">xss</a></p>
+<p><img src="data%3Atext/html;base64,PHNjcmlwdD5hbGVydCgxKTwvc2NyaXB0Pg==" alt="xss" /></p>
+<p><img src="data%3Atext/html;base64,PHNjcmlwdD5hbGVydCgxKTwvc2NyaXB0Pg==" alt="xss" /></p>
+<p><img src="data%3A//text/html;base64,PHNjcmlwdD5hbGVydCgxKTwvc2NyaXB0Pg==" alt="xss" /></p>
+<p><img src="data&amp;colon;text/html;base64,PHNjcmlwdD5hbGVydCgxKTwvc2NyaXB0Pg==" alt="xss" /></p>
diff --git a/test/data/xss_bad_url.md b/test/data/xss_bad_url.md
@@ -0,0 +1,31 @@
+[xss](javascript:alert(1))
+
+[xss]( javascript:alert(1))
+
+[xss](javascript://alert(1))
+
+[xss](javascript&colon;alert(1))
+
+![xss](javascript:alert(1))
+
+![xss]( javascript:alert(1))
+
+![xss](javascript://alert(1))
+
+![xss](javascript&colon;alert(1))
+
+[xss](data:text/html;base64,PHNjcmlwdD5hbGVydCgxKTwvc2NyaXB0Pg==)
+
+[xss]( data:text/html;base64,PHNjcmlwdD5hbGVydCgxKTwvc2NyaXB0Pg==)
+
+[xss](data://text/html;base64,PHNjcmlwdD5hbGVydCgxKTwvc2NyaXB0Pg==)
+
+[xss](data&colon;text/html;base64,PHNjcmlwdD5hbGVydCgxKTwvc2NyaXB0Pg==)
+
+![xss](data:text/html;base64,PHNjcmlwdD5hbGVydCgxKTwvc2NyaXB0Pg==)
+
+![xss]( data:text/html;base64,PHNjcmlwdD5hbGVydCgxKTwvc2NyaXB0Pg==)
+
+![xss](data://text/html;base64,PHNjcmlwdD5hbGVydCgxKTwvc2NyaXB0Pg==)
+
+![xss](data&colon;text/html;base64,PHNjcmlwdD5hbGVydCgxKTwvc2NyaXB0Pg==)
diff --git a/test/data/xss_text_encoding.html b/test/data/xss_text_encoding.html
@@ -0,0 +1,7 @@
+<p>&lt;script&gt;alert(1)&lt;/script&gt;</p>
+<p>&lt;script&gt;</p>
+<p>alert(1)</p>
+<p>&lt;/script&gt;</p>
+<p>&lt;script&gt;
+alert(1)
+&lt;/script&gt;</p>
diff --git a/test/data/xss_text_encoding.md b/test/data/xss_text_encoding.md
@@ -0,0 +1,12 @@
+<script>alert(1)</script>
+
+<script>
+
+alert(1)
+
+</script>
+
+
+<script>
+alert(1)
+</script>