Magento Question posted in
The official documentation can be found here.

Do i need to avoid sql injection in this example magento code?

Following source codes need to avoid sql-injection?
if $some_text of example is sql-injected attack, following source codes are dangerous?

  1. General Magento code

    $tmp_sale_info_collection = Mage::getModel('some/module')
    ->getCollection()
    ->addFieldToFilter('seller_id', array('eq' => $some_text));

  2. Use getSelect() inner join

    $orderItem = Mage::getModel('sales/order_item')->getCollection();
$orderItem->getSelect()
    ->joinInner(
        array(
            'order' => Mage::getSingleton('core/resource')->getTableName('sales/order')
        ), 
        'order.entity_id = main_table.order_id'
    )
    ->where('product_id=?', $some_text)
    ->order('main_table.order_id DESC');

  3. Use fetchAll() style1

    $select = $adapter->select()
    ->from($table, array())
    ->where($entityTypeIdField . ' =?', $some_text)
    ->where('attribute_id =?', $some_text)
    ->where('store_id =?', $some_text)
    ->columns('*');

$values = $adapter->fetchAll($select);

  4. Use fetchAll() style2

    $sql_select = "SELECT * from onetable where from_id ='$some_text'";
$resource = Mage::getModel('core/resource');
$read = $resource->getConnection('core_read');
$results = $read->fetchAll($sql_select);

Which one is dangerous and which is not?

=================== EDITED ===========================

  1. modified second

    $orderItem = Mage::getModel(‘sales/order_item’)->getCollection();
    $orderItem->getSelect()
    ->joinInner(
    array(
    ‘order’ => Mage::getSingleton(‘core/resource’)->getTableName(‘sales/order’)
    ),
    ‘order.entity_id = ‘ . $some_text . ‘
    )
    ->where(‘product_id=?’, $some_text)
    ->order(‘main_table.order_id DESC’);

Tags:

2

Answers


  1. 0

    I think you should not use full SQL statement. So, should not use “(4)Use fetchAll() style2“

    Login or Signup to reply.
  2. 0

    Magento Use prepared statements with bound variables which supported by the database. They are provided by PDO, by MySQLi and by other libraries.
    If the database layer doesn’t support binding variables then quote each non numeric user supplied value that is passed to the database with the database-specific string escape function like,mysql_real_escape_string(), addslashes(), magic_quotes_gpc, get_magic_quotes_gpc(), stripslashes(), htmlentities, htmlspecialchars.

    Just for you example :

    Here , I have used your first query which is :

    $productSku = array('101_7898_2200');
//$productSku = array('" OR ""="');//  sku= "' OR ''='"; And sku= '" OR ""="';

$attributes = Mage::getSingleton('catalog/config')->getProductAttributes();
$collection = Mage::getModel('catalog/product')
            ->getCollection()                
            ->addAttributeToFilter('sku', array('eq' => $productSku));
           // ->addAttributeToSelect($attributes);

echo "<pre> product collection query="; print_r($collection->getSelect()->__toString()); 
echo "<pre> product collection data="; print_r($collection->getData());

    Here SQL Query is :

    SELECT 1 AS `status`, `e`.`entity_id`, `e`.`type_id`, `e`.`attribute_set_id`, `e`.`sku` FROM `catalog_product_flat_1` AS `e` WHERE (e.sku = '101_7898_2200')

    OUTPUT collection is ::

    Array
(
[0] => Array
    (
        [status] => 1
        [entity_id] => 22835
        [type_id] => configurable
        [attribute_set_id] => 9
        [sku] => 101_7898_2200
    )

)

    BUT when I passed sql injected parameter which is sku= “‘ OR ”='”; And sku= ‘” OR “”=”‘;
    thereafter, we get the below result :

    SQL query is ::

    SELECT 1 AS status, e.entity_id, e.type_id, e.attribute_set_id, e.sku FROM catalog_product_flat_1 AS e WHERE (e.sku = ‘” OR “”=”‘)

    OUTPUT collection is :: Nothing

    product collection data=Array
    (
    )

    The best advice to avoid SQL injection vulnerability is “do not directly query the database”. You should be using the ORM which would protect you in these situations. Especially when grabbing data out of the EAV tables.

    However, if you are running a native sql query with parameter input, you should bind the query parameters to the query with Zend_Db_Select’s bind rather than using a full SQL statement:

    $query = $this->_connection->select()->from(‘eav_attribute’)->where(‘attribute_id=?’, $attributeId); $result = $this->_connection->fetchAll($query);

    Login or Signup to reply.
Please signup or login to give your own answer.

Back To Top
Search