Sonarqube cloud sonar-scanner not catching XSS vulnerabilities

SonarQube Cloud
, ,
1

  • Bitbucket Cloud

  • Languages of the repository: Php, Html, Javascript, css, etc.

  • We have a large project that we ran the sonar-scanner manually on and it did not find any XSS vulnerabilities, even though there are many in our phtml files. An example of something that poses a XSS vulnerability but was not caught is below.

  • <option value="<?php echo $layoutId; ?>"><?php echo $name; ?></option>

  • In the above snippet it should warn us that name is not escaped. We are wondering why the scanner is not picking up any of these?

1 Like
4

Hi @Hallie_Oberg

Thank you for your question, and welcome to the community!

The *.phtml files are analyzed by default by the sonar-php analyzer. Could you please share a minimum example of the whole file? This will help me to verify it.

Do you have any particular rule in mind that should be raised in your example? You can search for rules here: https://rules.sonarsource.com/

Best
Marcin Stachniuk

6

This rule should be triggered: PHP static code analysis. We have some very obvious values that are not escaped (using htmlspecialchars()) and none of them are triggering this rule. This makes the analyzer very unhelpful for our use case as we are hoping to find security vulnerabilities across our application. Here is a few more examples of what I would expect to trigger the rule.

// $customer['value'] and $customer['label'] are not escaped (could be user defined and include xss) 
 <legend><span>1</span> <?php echo $this->t('Select Account'); ?></legend>
 <div class="customer-select">
    <select id="customer-list" name="customer-list" aria-label="Account" value="">
        <option value="">-- Select an account --</option>
            <?php foreach($customers as $customer){
                   echo '<option value="' . $customer['value'] . '">' . $customer['label'] . '</option>';}
            ?>
   </select>
</div>


// $title is not escaped (could be user defined and include xss) 
<h4 class='page-name'><?php echo $title; ?></h4>
7

Thanks for the more context.

  1. What does the file header look like? The code snippet suggests that the file contains multiple <?php .... ?> tags. Is there <?php tag at the beginning?
  2. What is a file extension? Is it one of: php, php3, php4, php5, phtml, inc?

Best
Marcin

8
  1. The file header does usually start with a <?php tag like this:
<?php $this->layout('template', array(
    'title' => $this->t('Account Home') ,
)); ?>
  1. The file extension is .phtml
11

Hello @Hallie_Oberg, thanks for reaching out with these details.

In the examples you provided, you mention “could be user defined and include xss”. How do you know that it can be user defined?

// $title is not escaped (could be user-defined and include xss) 
<h4 class='page-name'><?php echo $title; ?></h4>

Can you also provide the part that assigns a user-controlled value to title, for instance?

I’m asking this because I suspect the rule is not able to identify where the variables are assigned something user-controlled.

Cheers,
Quentin

14

I suspect the issue is that the scanner is unable to determine if a variable is user defined because it comes from a controller method that fetches data from the database. The variable is user defined on a separate page and seperate controller method that inserts it into the database. So in other words it is very nested. Here is a simple example of a controller method that renders the page admin/main.phtml and returns a user defined value from the database that is then echoed in the phtml file:

/**
     * Index.
     *
     * @param \Psr\Http\Message\ServerRequestInterface $request
     * @param \Psr\Http\Message\ResponseInterface      $response
     * @param callable                                 $next
     *
     * @return \Psr\Http\Message\ResponseInterface
     */
    public function indexAction(ServerRequestInterface $request, RequestHandlerInterface $handler)
    {

        $cid = $_SESSION['CID'];
        $customerService = $this->getContainer()->get('PacPow\Service\Customer');
        $customer = $customerService->findById($cid);

        return new HtmlResponse(
            $this->render('admin/main', ['currentCustomer' => $customer->getCustomerName()])
        );
    }

<h4 class='customer-name'><?php echo $currentCustomer; ?></h4>

I have found that other static code analyzers are not able to catch things like this either, so I am not sure that there is a solid solution. If you have any ideas for work arounds let me know, otherwise, thank you for your time.

15

Hey Hallie!

This is what we call a second-order injection, as it does not come directly from the user. In theory, it is also possible to detect such issues, but their nature makes them very prone to false positives, so we did not add support for it yet.

In the Enterprise Edition of the on-premises version of SonarQube, you can add custom sources, which could be used to mark responses from the database as tainted. However, this feature is not available in SonarQube Cloud.

For better results (in general), I would also recommend using auto-wiring to inject the services into the classes instead of fetching them with reflection because it is difficult for a static analyzer to resolve reflection (it happens at run time, but static analyzers don’t run the code). Alternatively, a phpDoc comment could be used to specify the type of $customerService but I think auto-wiring is the better solution. This will also improve the results in the IDE, e.g., from PhpStorm.

1 Like