Following source codes need to avoid sql-injection
?
if $some_text
of example is sql-injected
attack, following source codes are dangerous?
-
General Magento code
$tmp_sale_info_collection = Mage::getModel('some/module') ->getCollection() ->addFieldToFilter('seller_id', array('eq' => $some_text));
-
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');
-
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);
-
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 ===========================
-
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’);
2
Answers
I think you should not use full SQL statement. So, should not use “(4)Use fetchAll() style2“
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 :
Here SQL Query is :
OUTPUT collection is ::
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
FROMcatalog_product_flat_1
ASe
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);