1
trabis
My search 1.1 SQL Injection vulnerable
  • 2009/3/13 1:18

  • trabis

  • Core Developer

  • Posts: 2269

  • Since: 2006/9/1 1


I found a problem on this this module, it is vulnerable to SQL injection!

Please open file class/searches.php and replace around line 131:
$format "INSERT INTO %s (mysearchid, keyword, datesearch, uid, ip) VALUES (%u, %s, %s, %u, %s)";
            
$sql sprintf($format ,
                            
$this->db->prefix('mysearch_searches'),
                            
$this->db->genId($this->db->prefix("mysearch_searches")."_mysearchid_seq"),
                            
$this->db->quoteString($keyword),$this->db->quoteString($datesearch),
                            
$uid,
                            
$this->db->quoteString($ip));
            
$force true;
        } else {
            
$format "UPDATE %s SET keyword=%d, datesearch=%s, uid=%u, ip=%s WHERE mysearchid = %u";
            
$sql sprintf($format,
                            
$this->db->prefix('mysearch_searches'),
                            
$this->db->quoteString($keyword),
                            
$this->db->quoteString($datesearch),
                            
$uid,
                            
$this->db->quoteString($ip),
                            
$mysearchid);
        }


Sorry for the inconvenience, I'll submit this fix ASAP.

2
Anonymous
Re: My search 1.1 SQL Injection vulnerable
  • 2009/3/13 1:59

  • Anonymous

  • Posts: 0

  • Since:


Please submit this bug to Herve since mysearch based on isearch module.

3
anderssk
Re: My search 1.1 SQL Injection vulnerable
  • 2009/3/13 6:31

  • anderssk

  • Quite a regular

  • Posts: 335

  • Since: 2006/3/21


THX for sharing that information

posted on xoopsnordic

4
trabis
Re: My search 1.1 SQL Injection vulnerable
  • 2009/3/13 13:30

  • trabis

  • Core Developer

  • Posts: 2269

  • Since: 2006/9/1 1


Quote:

Mowaffaq wrote:
Please submit this bug to Herve since mysearch based on isearch module.


Confirmed, iSearch 1.8 is also vulnerable. The fix is the same.

5
hervet
Re: My search 1.1 SQL Injection vulnerable
  • 2009/3/13 14:03

  • hervet

  • Friend of XOOPS

  • Posts: 2267

  • Since: 2003/11/4


I could be wrong but :

1/ Data are coming from XOOPS itself, so sanitized and they are sanitzed by the module by this code :
if (!$searches->cleanVars()) {
            foreach(
$searches->getErrors() as $oneerror) {
                echo 
"

".$oneerror."

"
;
            }
            return 
false;
        }

Just before the code you mention.

2/ If there is an Sql injection, quoteString will not change anything.

6
trabis
Re: My search 1.1 SQL Injection vulnerable
  • 2009/3/13 14:15

  • trabis

  • Core Developer

  • Posts: 2269

  • Since: 2006/9/1 1


Hum , well, there is something in xoops.

This only happens if using "magic quotes gpc" in php settings.

However, using db->quotestring solves the problem whatever the value of "magic quotes gpc".

So this is something we should look into, I guess $ts->stripSlashesGPC is not doing a good job.

7
hervet
Re: My search 1.1 SQL Injection vulnerable
  • 2009/3/13 15:10

  • hervet

  • Friend of XOOPS

  • Posts: 2267

  • Since: 2003/11/4


Quote:

trabis wrote:
This only happens if using "magic quotes gpc" in php settings.

no

8
trabis
Re: My search 1.1 SQL Injection vulnerable
  • 2009/3/13 19:04

  • trabis

  • Core Developer

  • Posts: 2269

  • Since: 2006/9/1 1


Quote:

hervet wrote:
Quote:

trabis wrote:
This only happens if using "magic quotes gpc" in php settings.

no


Oh yes,
Here is what happens:

When a search is made, slashes are added to $queryarray[$i] to prevent SQL INJECTION (using 'addslashes' method).

Then isearch uses setVar('keyword',$queryarray[$i]);
which is an already slashed keyword;

When latter you use cleanVars the object class presumes that vars have not been slashed and, if the server is auto adding slashes with 'magic quotes gpc', the object will clean the var by removing the slashes.

So, at this point $queryarray[$i] is not slashed anymore and can easily break the query if you search for

test'

for example

How to fix?

You can use quotestring for adding slashes and quoting the string so it can be safely used in the query.

Or you can use

$isearch->setVar('keyword',$queryarray[$i], true);

Where true means:
"Hey, this string already has slashes so please, do not remove them".

So this is not a core bug, it is a module bug.

Login

Who's Online

345 user(s) are online (273 user(s) are browsing Support Forums)


Members: 0


Guests: 345


more...

Donat-O-Meter

Stats
Goal: $100.00
Due Date: Nov 30
Gross Amount: $0.00
Net Balance: $0.00
Left to go: $100.00
Make donations with PayPal!

Latest GitHub Commits