15

When joining tables, I use SQL models of the Zend Framework. As example I modified my actual code, but I think you will get the point:

$this->getSelect()->join(
                      array('sections' => $sectionsTableName),
                      'main_table.banner_id = pages.banner_id',
                      array()
                    )
                  ->where("sections.section= '$section' OR sections.section = '0' OR (sections.section = '6' AND ? LIKE main_table.url)",$url)
                  ->group('main_table.banner_id'); 

The page is loaded with ajax and $section parameter is sent as GET parameter (www.example.com/controllerName/index/display/3?paremeter1=example&section=www.example2.com).

Now here is the problem if somebody perform something like this:

www.example.com/controllerName/index/display/3?paremeter1=example&url=(SELECT 3630 FROM(SELECT COUNT(*),CONCAT(0x7170786a71,(SELECT (ELT(3630=3630,1))),0x717a716b71,FLOOR(RAND(0)*2))x FROM INFORMATION_SCHEMA.CHARACTER_SETS GROUP BY x)a)

In this way user can dump entire database. The data won't be displayed, but still SQL will perform dump which can cause sql overload.

Questions:

  1. What is the best way to prevent such scenario?
  2. Now I am worried for previous customers. Is with this code possible to make even more risk action, like delate or alter table? I guess not because you can not put any other statement than SELECT inside subselect so DELETE would produce sql syntax error. Am I right?

UPDATE: My example is not proper illustration of SQL injection because there is ' sign arround $sections and thus it won't be possible to make injection. Anyway this would be possible when expecting integer value and when you don't filter integer input. See my comment below.

  • 1
    You could use: $db = Mage::getSingleton('core/resource')->getConnection('core_read'); and $db->quote() even in your case look at $db->quoteInto. If $this is a resource, you could do: $this->getConnection('core_read')->quoteInto() if it's a collection you could do: $this->getResource()->getConnection('core_read')->quoteInto(). along those lines. If that helps guide you towards your goal. – ash Jun 9 '15 at 16:24
  • I just realized that this scenario is possible only if value is integer. If value is varchar, then there will be always ' sign before ( sign and thus (SELECT or anything else will be just as string and not function. When field is integer, then ' is not needed and it makes possible such scenario. But integer should always be filtered with intval() so this is also not issue. – JohnyFree Jun 9 '15 at 16:50
  • What if you start with closing the '? So ' AND (SELECT ...) '? By the way, I don't think Zend is not quoting this... And if you use bindings, then PDO will handle this. Just never use sting concatenations like this: "sections.section= '$section'" – 7ochem Jun 10 '15 at 7:36
  • @7ochem in such case you MUST to bind parameter using ? and ' will become \'. But if you use integer value, then you don't to bind it since you can clean it using intval() php function and 'something will become 0. – JohnyFree Jun 17 '15 at 17:43
8

Validate your input!

As good and as much as you can.

Some suggestions for your validation:

  1. Check the length of the variable you get via GET-parameter. There's no need to accept a neverending long string.

  2. Validate for a domain name. What kind of format do your expected domain names have? Is it always www.mydomain.tld? Create a regex that checks for a match or (better) use Zend_Validate_Hostname:

    $validator = new Zend_Validate_Hostname();
    if ($validator->isValid($hostname)) {
        //hostname is valid - continue
    }
    
  3. Whitelisting: Do you know which domainnames to expect? You could create a list of allowed domains and check against them. Drop the rest.

    $allowedDomains = array('www.domain1.tld','www.domain2.tld');
    
  4. Blacklisting domainnames and or characters: If you expect a domain name, there's no need to accept any other characters than a-z and 0-9 and "." (unless you're working with special domain names).

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.