Improper Restriction of Rendered UI Layers or Frames in osticket/osticket

Valid

Reported on

Mar 29th 2023


Description

The osTicket uses an incorrect method to validate the src attribute of the iframe tag. Although it appears that osTicket restricts domains through a whitelist, attackers can easily bypass this restriction.

Proof of Concept

<iframe src="http://www.youtube.com.[attacker's server]">
This iframe is going to render www.youtube.com.[attacker's server]

Impact

Since the origin of an iframe is cross-origin, it cannot perform actions such as reading cookies from the actual server. However, it can execute arbitrary JavaScript code.

We are processing your report and will contact the osticket team within 24 hours. a year ago
We have contacted a member of the osticket team and are waiting to hear back a year ago
osticket/osticket maintainer
a year ago

Maintainer


@endansdto

My apologies, I meant to reply to you way earlier. Thank you for the report. Can you please provide a little more information on this report?

  • What version of osTicket are you targeting?
  • What is your Admin Panel > Settings > System > Embedded Domain Whitelist set to?
  • Can you provide an example of an attack?

Cheers.

Ivy
a year ago

Researcher


A1. I am targeting the latest version of osTicket, which is 1.17.3, provided by Github. A2. I am using the default settings provided by osTicket, which include youtube.com, dailymotion.com, vimeo.com, player.vimeo.com, and web.microsoftstream.com. A3. Here is my PoC video: https://www.youtube.com/watch?v=B4fN4sm-9HA

JediKev
10 months ago

Maintainer


Hello Ivy,

I have confirmed your report is valid. Below is a patch that should mitigate the vuln. Please test and get back to us with confirmation or further notes.

diff --git a/include/class.format.php b/include/class.format.php
index 88432f87..6066cd15 100644
--- a/include/class.format.php
+++ b/include/class.format.php
@@ -359,7 +359,7 @@ class Format {
             $config['elements'] .= '+iframe';
             $config['spec'] = 'iframe=-*,height,width,type,style,src(match="`^(https?:)?//(www\.)?('
                 .implode('|', $whitelist)
-                .')/?([^@]*)$`i"),frameborder'.($options['spec'] ? '; '.$options['spec'] : '').',allowfullscreen';
+                .')/?([^.]*[^@]*)$`i"),frameborder'.($options['spec'] ? '; '.$options['spec'] : '').',allowfullscreen';
         }
 
         return Format::html($html, $config);

Cheers.

JediKev
10 months ago

Maintainer


Actually, this might be better regex:

diff --git a/include/class.format.php b/include/class.format.php
index 88432f87..a3db5d8f 100644
--- a/include/class.format.php
+++ b/include/class.format.php
@@ -359,7 +359,7 @@ class Format {
             $config['elements'] .= '+iframe';
             $config['spec'] = 'iframe=-*,height,width,type,style,src(match="`^(https?:)?//(www\.)?('
                 .implode('|', $whitelist)
-                .')/?([^@]*)$`i"),frameborder'.($options['spec'] ? '; '.$options['spec'] : '').',allowfullscreen';
+                .')/?(?!\.)([^@]*)$`i"),frameborder'.($options['spec'] ? '; '.$options['spec'] : '').',allowfullscreen';
         }
 
         return Format::html($html, $config);
Ivy
10 months ago

Researcher


Hello @JediKev. In my opinion, it seems that the patch is not sufficient. I have confirmed that the same vulnerability still occurs in the code, including another domain. Please check the code below.

<iframe src="http://www.youtube.com[attacker's server]"> e.g., <iframe src="http://www.youtube.comm.211.229.218.12.nip.io:45555"></iframe>

JediKev
10 months ago

Maintainer


I see what you're saying. I will provide better regex soon.

JediKev
10 months ago

Maintainer


Here is better regex:

diff --git a/include/class.format.php b/include/class.format.php
index 88432f87..1c6ab211 100644
--- a/include/class.format.php
+++ b/include/class.format.php
@@ -359,7 +359,7 @@ class Format {
             $config['elements'] .= '+iframe';
             $config['spec'] = 'iframe=-*,height,width,type,style,src(match="`^(https?:)?//(www\.)?('
                 .implode('|', $whitelist)
-                .')/?([^@]*)$`i"),frameborder'.($options['spec'] ? '; '.$options['spec'] : '').',allowfullscreen';
+                .')(\?|/|#)([^@]*)$`i"),frameborder'.($options['spec'] ? '; '.$options['spec'] : '').',allowfullscreen';
         }
 
         return Format::html($html, $config);

This requires the host to be followed by a path (/), query (?), or fragment (#) character.

Ivy
10 months ago

Researcher


The patch seems appropriate and no more existing vulnerabilities that bypass filtering are expected to occur.

JediKev validated this vulnerability 8 months ago
endansdto has been awarded the disclosure bounty
The fix bounty is now up for grabs
The researcher's credibility has increased: +7
JediKev marked this as fixed in v1.17.4 with commit a22ff3 8 months ago
JediKev has been awarded the fix bounty
This vulnerability has now been published 8 months ago
class.format.php#L355L363 has been validated
to join this conversation