Score:1

Drupal Commerce Product Variation SQL Injection Vulnerability

gg flag

A PCI scan on my site is saying that there is a SQL Injection vulnerability. But where it seems to be indicating it is on pretty basic Commerce functionality. When visiting a product variation page directly, it appends v=nn to the URL to show the selected variation.

This scan is suggesting that URLs like:

?v=54+or+5459%3D5459&page=1

?v=54+or+6721%3D8812&page=1

... return TRUE and FALSE, respectively (return where, I'm not really sure).

This is all pretty default behavior of Drupal Commerce if I'm not mistaken. Is it possible that this is a false positive from this PCI scan?

I know I haven't added any custom queries to this site. Other than theming, it's pretty out-of-the-box behavior. And I don't think I'm using the v= parameter directly in any of my theming.

I'm sorry this is vague and general, but looking for some more information on how to address this PCI result.

Thanks in advance!

Score:3
us flag

?v=54+or+5459%3D5459&page=1 and ?v=54+or+6721%3D8812&page=1 would allow SQL injection if the code running the query would use the values passed in the URL without filtering them, as the following code, for example. (It's not the code the module is using. It's just code that would allow an SQL injection.)

$statement = "select * from product_variation pv WHERE pv.id = {$_GET['v']}"; 

The SQL queries would be essentially equivalent, respectively, to the following ones.

select * from product_variation pv where pv.id = 54 or 5459 = 5459
select * from product_variation pv where pv.id = 54 or 6721 = 8812

The first SQL query would return all the rows in the product_variation database table, while the second query would return only the row for which the ID is equal to 54.

Drupal core and contributed modules don't concatenate values passed from the user (even through the URL) with literal strings to build the SQL string, but use proper argument substitution, making SQL injection not possible.
They don't also allow users to provide any operator to a query's condition, but set a list of allowed operators and only allow users to use those.

The PCI scan noticed it got two different results (TRUE and FALSE) using ?v=54+or+5459%3D5459&page=1 and ?v=54+or+6721%3D8812&page=1, interpreted TRUE as there are rows matching the query and FALSE as there aren't rows matching the query, and thought the SQL injection was successful. (How the PCI scan can say that accessing URLs containing those query parameter would return TRUE and FALSE is not clear to me either.)

That is not what happened. What reported by the scan is a false positive.

mangohost

Post an answer

Most people don’t grasp that asking a lot of questions unlocks learning and improves interpersonal bonding. In Alison’s studies, for example, though people could accurately recall how many questions had been asked in their conversations, they didn’t intuit the link between questions and liking. Across four studies, in which participants were engaged in conversations themselves or read transcripts of others’ conversations, people tended not to realize that question asking would influence—or had influenced—the level of amity between the conversationalists.