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.

4
  • 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
    Commented Jun 9, 2015 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
    Commented Jun 9, 2015 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
    Commented Jun 10, 2015 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
    Commented Jun 17, 2015 at 17:43

1 Answer 1

9

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 and acknowledge you have read our privacy policy.

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