Skip to content

Commit

Permalink
Fix sqli ->escape after ->escapeforlike
Browse files Browse the repository at this point in the history
  • Loading branch information
eldy committed Nov 21, 2022
1 parent e480b7c commit 7c1eac9
Show file tree
Hide file tree
Showing 5 changed files with 194 additions and 12 deletions.
7 changes: 4 additions & 3 deletions htdocs/contact/list.php
Expand Up @@ -502,16 +502,17 @@
if ($value['active'] && strlen($search_[$key])) {
$searchkeyinjsonformat = preg_replace('/"$/', '', preg_replace('/^"/', '', json_encode($search_[$key])));
if (in_array($db->type, array('mysql', 'mysqli'))) {
$sql .= " AND p.socialnetworks REGEXP '\"".$db->escapeforlike($db->escape($key))."\":\"[^\"]*".$db->escapeforlike($db->escape($searchkeyinjsonformat))."'";
$sql .= " AND p.socialnetworks REGEXP '\"".$db->escape($db->escapeforlike($key))."\":\"[^\"]*".$db->escape($db->escapeforlike($searchkeyinjsonformat))."'";
} elseif ($db->type == 'pgsql') {
$sql .= " AND p.socialnetworks ~ '\"".$db->escapeforlike($db->escape($key))."\":\"[^\"]*".$db->escapeforlike($db->escape($searchkeyinjsonformat))."'";
$sql .= " AND p.socialnetworks ~ '\"".$db->escape($db->escapeforlike($key))."\":\"[^\"]*".$db->escape($db->escapeforlike($searchkeyinjsonformat))."'";
} else {
// Works with all database but not reliable because search only for social network code starting with earched value
$sql .= " AND p.socialnetworks LIKE '%\"".$db->escapeforlike($db->escape($key))."\":\"".$db->escapeforlike($db->escape($searchkeyinjsonformat))."%'";
$sql .= " AND p.socialnetworks LIKE '%\"".$db->escape($db->escapeforlike($key))."\":\"".$db->escape($db->escapeforlike($searchkeyinjsonformat))."%'";
}
}
}
}
//print $sql;
if (strlen($search_email)) {
$sql .= natural_search('p.email', $search_email);
}
Expand Down
13 changes: 8 additions & 5 deletions htdocs/core/lib/website.lib.php
Expand Up @@ -887,7 +887,7 @@ function getSocialNetworkSharingLinks()
* @param string $langcode Language code ('' or 'en', 'fr', 'es', ...)
* @param array $otherfilters Other filters
* @param int $status 0 or 1, or -1 for both
* @return string HTML content
* @return array Array with results of search
*/
function getPagesFromSearchCriterias($type, $algo, $searchstring, $max = 25, $sortfield = 'date_creation', $sortorder = 'DESC', $langcode = '', $otherfilters = 'null', $status = 1)
{
Expand Down Expand Up @@ -925,6 +925,8 @@ function getPagesFromSearchCriterias($type, $algo, $searchstring, $max = 25, $so
$found = 0;

if (!$error && (empty($max) || ($found < $max)) && (preg_match('/meta/', $algo) || preg_match('/content/', $algo))) {
include_once DOL_DOCUMENT_ROOT.'/website/class/websitepage.class.php';

$sql = 'SELECT wp.rowid FROM '.MAIN_DB_PREFIX.'website_page as wp';
if (is_array($otherfilters) && !empty($otherfilters['category'])) {
$sql .= ', '.MAIN_DB_PREFIX.'categorie_website_page as cwp';
Expand All @@ -934,7 +936,7 @@ function getPagesFromSearchCriterias($type, $algo, $searchstring, $max = 25, $so
$sql .= " AND wp.status = ".((int) $status);
}
if ($langcode) {
$sql .= " AND wp.lang ='".$db->escape($langcode)."'";
$sql .= " AND wp.lang = '".$db->escape($langcode)."'";
}
if ($type) {
$tmparrayoftype = explode(',', $type);
Expand All @@ -947,11 +949,11 @@ function getPagesFromSearchCriterias($type, $algo, $searchstring, $max = 25, $so
$sql .= " AND (";
$searchalgo = '';
if (preg_match('/meta/', $algo)) {
$searchalgo .= ($searchalgo ? ' OR ' : '')."wp.title LIKE '%".$db->escapeforlike($db->escape($searchstring))."%' OR wp.description LIKE '%".$db->escapeforlike($db->escape($searchstring))."%'";
$searchalgo .= ($searchalgo ? ' OR ' : '')."wp.keywords LIKE '".$db->escapeforlike($db->escape($searchstring)).",%' OR wp.keywords LIKE '% ".$db->escapeforlike($db->escape($searchstring))."%'"; // TODO Use a better way to scan keywords
$searchalgo .= ($searchalgo ? ' OR ' : '')."wp.title LIKE '%".$db->escape($db->escapeforlike($searchstring))."%' OR wp.description LIKE '%".$db->escape($db->escapeforlike($searchstring))."%'";
$searchalgo .= ($searchalgo ? ' OR ' : '')."wp.keywords LIKE '".$db->escape($db->escapeforlike($searchstring)).",%' OR wp.keywords LIKE '% ".$db->escape($db->escapeforlike($searchstring))."%'"; // TODO Use a better way to scan keywords
}
if (preg_match('/content/', $algo)) {
$searchalgo .= ($searchalgo ? ' OR ' : '')."wp.content LIKE '%".$db->escapeforlike($db->escape($searchstring))."%'";
$searchalgo .= ($searchalgo ? ' OR ' : '')."wp.content LIKE '%".$db->escape($db->escapeforlike($searchstring))."%'";
}
$sql .= $searchalgo;
if (is_array($otherfilters) && !empty($otherfilters['category'])) {
Expand All @@ -963,6 +965,7 @@ function getPagesFromSearchCriterias($type, $algo, $searchstring, $max = 25, $so
//print $sql;

$resql = $db->query($sql);

if ($resql) {
$i = 0;
while (($obj = $db->fetch_object($resql)) && ($i < $max || $max == 0)) {
Expand Down
4 changes: 2 additions & 2 deletions htdocs/core/modules/import/import_csv.modules.php
Expand Up @@ -862,8 +862,8 @@ public function import_insert($arrayrecord, $array_match_file_to_database, $obji
$stringtosearch = json_encode($socialnetwork).':'.json_encode($json->$socialnetwork);
//var_dump($stringtosearch);
//var_dump($this->db->escape($stringtosearch)); // This provide a value for sql string (but not for a like)
$where[] = $key." LIKE '%".$this->db->escapeforlike($this->db->escape($stringtosearch))."%'";
$filters[] = $col." LIKE '%".$this->db->escapeforlike($this->db->escape($stringtosearch))."%'";
$where[] = $key." LIKE '%".$this->db->escape($this->db->escapeforlike($stringtosearch))."%'";
$filters[] = $col." LIKE '%".$this->db->escape($this->db->escapeforlike($stringtosearch))."%'";
//var_dump($where[1]); // This provide a value for sql string inside a like
} else {
$where[] = $key.' = '.$data[$key];
Expand Down
4 changes: 2 additions & 2 deletions htdocs/core/modules/import/import_xlsx.modules.php
Expand Up @@ -908,8 +908,8 @@ public function import_insert($arrayrecord, $array_match_file_to_database, $obji
$stringtosearch = json_encode($socialnetwork).':'.json_encode($json->$socialnetwork);
//var_dump($stringtosearch);
//var_dump($this->db->escape($stringtosearch)); // This provide a value for sql string (but not for a like)
$where[] = $key." LIKE '%".$this->db->escapeforlike($this->db->escape($stringtosearch))."%'";
$filters[] = $col." LIKE '%".$this->db->escapeforlike($this->db->escape($stringtosearch))."%'";
$where[] = $key." LIKE '%".$this->db->escape($this->db->escapeforlike($stringtosearch))."%'";
$filters[] = $col." LIKE '%".$this->db->escape($this->db->escapeforlike($stringtosearch))."%'";
//var_dump($where[1]); // This provide a value for sql string inside a like
} else {
$where[] = $key.' = '.$data[$key];
Expand Down
178 changes: 178 additions & 0 deletions test/phpunit/Website.class.php
@@ -0,0 +1,178 @@
<?php
/* Copyright (C) 2010 Laurent Destailleur <eldy@users.sourceforge.net>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
* or see https://www.gnu.org/
*/

/**
* \file test/phpunit/WebsiteTest.php
* \ingroup test
* \brief PHPUnit test
* \remarks To run this script as CLI: phpunit filename.php
*/

global $conf,$user,$langs,$db;
//define('TEST_DB_FORCE_TYPE','mysql'); // This is to force using mysql driver
//require_once 'PHPUnit/Autoload.php';

if (! defined('NOREQUIRESOC')) {
define('NOREQUIRESOC', '1');
}
if (! defined('NOCSRFCHECK')) {
define('NOCSRFCHECK', '1');
}
if (! defined('NOTOKENRENEWAL')) {
define('NOTOKENRENEWAL', '1');
}
if (! defined('NOREQUIREMENU')) {
define('NOREQUIREMENU', '1'); // If there is no menu to show
}
if (! defined('NOREQUIREHTML')) {
define('NOREQUIREHTML', '1'); // If we don't need to load the html.form.class.php
}
if (! defined('NOREQUIREAJAX')) {
define('NOREQUIREAJAX', '1');
}
if (! defined("NOLOGIN")) {
define("NOLOGIN", '1'); // If this page is public (can be called outside logged session)
}
if (! defined("NOSESSION")) {
define("NOSESSION", '1');
}

require_once dirname(__FILE__).'/../../htdocs/main.inc.php';
require_once dirname(__FILE__).'/../../htdocs/core/lib/website.lib.php';


if (empty($user->id)) {
print "Load permissions for admin user nb 1\n";
$user->fetch(1);
$user->getrights();
}
$conf->global->MAIN_DISABLE_ALL_MAILS=1;


/**
* Class for PHPUnit tests
*
* @backupGlobals disabled
* @backupStaticAttributes enabled
* @remarks backupGlobals must be disabled to have db,conf,user and lang not erased.
*/
class WebsiteTest extends PHPUnit\Framework\TestCase
{
protected $savconf;
protected $savuser;
protected $savlangs;
protected $savdb;

/**
* Constructor
* We save global variables into local variables
*
* @return SecurityTest
*/
public function __construct()
{
parent::__construct();

//$this->sharedFixture
global $conf,$user,$langs,$db;
$this->savconf=$conf;
$this->savuser=$user;
$this->savlangs=$langs;
$this->savdb=$db;

print __METHOD__." db->type=".$db->type." user->id=".$user->id;
//print " - db ".$db->db;
print "\n";
}

/**
* setUpBeforeClass
*
* @return void
*/
public static function setUpBeforeClass()
{
global $conf,$user,$langs,$db;
$db->begin(); // This is to have all actions inside a transaction even if test launched without suite.

print __METHOD__."\n";
}

/**
* tearDownAfterClass
*
* @return void
*/
public static function tearDownAfterClass()
{
global $conf,$user,$langs,$db;
$db->rollback();

print __METHOD__."\n";
}

/**
* Init phpunit tests
*
* @return void
*/
protected function setUp()
{
global $conf,$user,$langs,$db;
$conf=$this->savconf;
$user=$this->savuser;
$langs=$this->savlangs;
$db=$this->savdb;

print __METHOD__."\n";
}

/**
* End phpunit tests
*
* @return void
*/
protected function tearDown()
{
print __METHOD__."\n";
}


/**
* testGetPagesFromSearchCriterias
*
* @return void
*/
public function testGetPagesFromSearchCriterias()
{
global $db;

$s = "123') OR 1=1-- \' xxx";
/*
var_dump($s);
var_dump($db->escapeforlike($s));
var_dump($db->escape($db->escapeforlike($s)));
*/

$res = getPagesFromSearchCriterias('page,blogpost', 'meta,content', $s, 2, 'date_creation', 'DESC', 'en');
//var_dump($res);
print __METHOD__." message=".$res['code']."\n";
// We must found no line (so code should be KO). If we found somethiing, it means there is a SQL injection of the 1=1
$this->assertEquals($res['code'], 'KO');
}
}

0 comments on commit 7c1eac9

Please sign in to comment.