SonarPHP security good practices

php

(Pierre-Loup Tristant) #1

Hi,

I have been working on a few security rules for SonarPHP.
Those rules might not perform as well as the ones used in SAST software (no taint or call flow analyses) but they help enforcing security good practices.

Some rules have been designed for Drupal 7 & Drupal 8 codebase.

Here is a list of the rules already written:
PHP

  • Functions used for system command execution are forbidden.
  • TLS trust chain verification should not be disabled.
  • TLS configuration should not be changed dynamically.
  • Namespace importing should be preferred over include/require functions.
  • unserialize function should not be used on untrusted data.
  • Superglobal variable that contains user input should not be used directly. Prefer using the filter_input() function.
  • filter_input() should not be used with FILTER_DEFAULT nor FILTER_UNSAFE_RAW filter option.

Drupal

  • Drupal static query should use proper argument substitution.
  • Drupal dynamic query should use proper argument substitution.
  • Drupal Form API unsanitized user input should not be used.

Are those types of rules wanted on the SonarPHP rules set?

Regards,
Pierre-Loup


(Alexandre Gigleux) #2

Hello Pierre-Loup,

I can recognize just by looking at the title some rules similar to the ones we are working on at SonarSource for PHP but also for other languages.

Do you have a public repository where I can look at your implementation to get more details about what these rules are catching, then we could decide to provide similar feature natively in SonarPHP.
Also, if you have a way to provide explanations what is the risk covered by each rules, that would be awesome to help developers understand what will be the impact if they don’t follow your recommendations.

Also, do not hesitate to dump your additional rule ideas not yet implemented here: https://community.sonarsource.com/c/suggestions/rules

Thanks


(Pierre-Loup Tristant) #3

Hello @Alexandre_Gigleux ,

I have no public repository yet but I will publish the source code in the next days on my company’s GitHub.
I’ll keep you informed.

I had a look at the new security rules visible on SonarPHP source code on GitHub.
I noticed this pull request : https://github.com/SonarSource/sonar-php/pull/307

It’s a custom rule I have also writen. I think the SonarPHP GitHub’s implementation is incomplete : it only detects the exec function usage. More than one PHP funtion can be used to execute OS commands :

  • passthru
  • proc_open
  • popen
  • shell_exec
  • system
  • pcntl_exec
  • `` (the “backticks” PHP Operator)

I am not sure which channel I should use to contibute and discus the new rules impementations. Should I add a comment on the closed pull request ? Should I open an issue ? Or should I add a comment on JIRA ?

Thanks.


(Alexandre Gigleux) #4

Hello,

I believe the best is to open one thread on the “Suggest new features” in this forum. One thread for one rule to keep things clean.

For “Rule S4721: Executing OS commands is security-sensitive”, we are going to adjust the implementation, no need to do an extra post.

Thanks


(Pierre-Loup Tristant) #5

Hello,

@Alexandre_Gigleux, I have added rules sugestions. You can find rules implementations here (some rules need to be reworked/improved).

Regards,
Pierre-Loup


(Tibor Blenessy) #6

A post was split to a new topic: No issue detected in “Damn Vulnerable Web App”