Syntax error or access violation: 1064 ' brandname

Question

I'm getting this error in my Magento script:

Product not added exception:exception 'PDOException' with message

'SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 's Secret'' at line 1'

Some background info:

I have a PHP script running on a cron job to add and update products. It runs a while now, but I got just now this error. I think it's because the manufacturers name got an apostrophe in it. But I have no clue how to fix it.

Changing the manufacturer's name is not a option.

function addManufacture($pid,$men){
    $resource = Mage::getSingleton('core/resource');
    $readConnection = $resource->getConnection('core_read');
    $query = "SELECT manufacturers_id FROM p1_manufacturers WHERE m_name='".$men."'";
    $lastid = $readConnection->fetchOne($query); 
    $write = Mage::getSingleton("core/resource")->getConnection("core_write");
    if($lastid){}else{
    $url = createUrl($men); 
    $query = "insert into p1_manufacturers (m_name,identifier,status) values ('".$men."','".$url."',1)";
    $write->query($query);
    $lastid = $write->lastInsertId();
    }
    $query1 = "insert into p1_manufacturers_products (manufacturers_id,product_id) values ('".$lastid."','".$pid."')";
    $write->query($query1);

    $query3 = "SELECT manufacturers_id FROM p1_manufacturers_store WHERE manufacturers_id='".$lastid."'";
    $mid = $readConnection->fetchOne($query3); 
    if($mid){} else {
    $query2 = "insert into p1_manufacturers_store (manufacturers_id,store_id) values ('".$lastid."',0)";
    $write->query($query2);
    }

}

Are you using prepared statements? That should fix any error caused by `'` and similar. — Sirko, Apr 06 '14 at 13:17
That breakage on a `'` single quote implies a SQL injection vulnerability. Start [learning how to use `prepare()/execute()` in PDO](http://www.php.net/manual/en/pdo.prepare.php), which is the preferred and secure way to use it. — Michael Berkowski, Apr 06 '14 at 13:18
There are examples in [How can I prevent SQL injection in PHP](http://www.php.net/manual/en/pdo.prepare.php) — Michael Berkowski, Apr 06 '14 at 13:19
Where's your code? At a guess, at the very least you probably need to escape the user input before sending your query. Most likely you have rampant SQL injection vulnerabilities and you should be considering a move to prepared statements. — , Apr 06 '14 at 13:20
Hi there. Whenever you ask a question, please add all necessary information in order to help us help you. Here, at a minimum, the database query that gives rise to the error would be essential, but the PHP code that runs it would probably be useful too. — halfer, Apr 06 '14 at 13:23
@Tommie: that's a good start. Now this introduces something called `Mage` but we cannot see the definition of it. Is this widely available, so you can link to a copy of it in your post? What software does this come from? — halfer, Apr 06 '14 at 13:58
@Tommie - [this question](http://stackoverflow.com/q/3575160/1864610) covers similar ground. — , Apr 06 '14 at 14:13

halfer · Answer 1 · 2014-04-06T13:57:10.513

Here is the problem:

$query = "SELECT manufacturers_id FROM p1_manufacturers WHERE m_name='".$men."'";

Replace that with:

$menEscaped = mysql_real_escape_string($men);
$query = "SELECT manufacturers_id FROM p1_manufacturers WHERE m_name='".$menEscaped."'";

For readability, I might be inclined to reformat it thus:

$menEscaped = mysql_real_escape_string($men);
$query = "
    SELECT
        manufacturers_id
    FROM
        p1_manufacturers
    WHERE
        m_name='{$menEscaped}'
";

The problem is you are not escaping your input variables, and if this comes from user input, you may find people injecting SQL of their own choice into your database. And that's generally not good!

Addendum: the above may work, but I've just spotted you are using a library called Mage. This being the case, you will need to find out how to escape strings using that library - it will be something like $write->escapeString($men).

As has been noted in the comments, it is even better if you can switch to paramerisation. You'll need to check if your library supports that.

@halfler First of all thanks for your input. Im currently testing it ;) Will keep you in the loop with my findings — Socket1, Apr 06 '14 at 14:03

score 1 · Answer 2 · edited May 23 '17 at 10:26

1

Your problem is being caused by an unescaped single-quote appearing in your data, and creating a syntax error in the queries you are submitting to your database.

Unfortunately, your database access code is hidden in some class, so it's not immediately obvious what changes are required. However...

As an absolute minimum you should escape any user data before applying it to the database. For this function this means

$men = mysql_real_escape_string($men);
$pid = mysql_real_escape_string($pid);

added at the top of the function. I have assume you are using 'mysql()` in this code.

Watch the line $url = createUrl($men); as this will be affected by this change. You may need further modifications here, and createUrl() may need to be changed too.

You will need to make similar changes in every function that accesses your database.

If you are using mysqli() more work will be required as the arguments are different and this 'easy' fix won't work.

Ultimately you should look to rewrite your code to use prepared statements.

Your code is seriously vulnerable to attack. There is a lot of work here. Get to it!

Edit Thanks to @halfer for spotting the use of Mage. Magento uses the Zend framework which in turn uses PDO objects. Delving into the code you can rewrite the functions to use prepared statements which will deal with your problem effectively. This answer has a fuller description. This is a better fix than I suggested above, but you still have a great deal of work to do.

edited May 23 '17 at 10:26

Community

1
1

answered Apr 06 '14 at 13:57

+1 from me! However, we both fell into the same trap. It is possible that the library in use does not use `mysql_` calls internally, and if so we should avoid mixing in a `mysql_` call. (Tommie, the reason we need to be careful with `mysql_real_escape_string` is that it requires an open connection to work reliably, and if you are using a different library the `mysql` library will believe the connection is closed). – halfer Apr 06 '14 at 14:00
1

@halfer I commented on the mysqli possibility. See my edit - I found a better answer already on [so] – Apr 06 '14 at 14:09
That did not do the trick :( exception:exception 'PDOException' with message 'SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 's Secret','victoria-#039;s-secret',1)' at line 1' in /public_html/lib/Zend/Db/Statement/Pdo.php:249 – Socket1 Apr 06 '14 at 14:44
I changed my code. Now this error popped: Fatal error: Call to undefined function mysql_real_ecape_string() in /home2/../public_html/crone/addproduct.php on line 320 I think i have to hire a dev to look at it... – Socket1 Apr 06 '14 at 14:52
@Tommie If you're still getting the syntax error you haven't constructed your query properly. You don't need mysql_real_escape_string() if you've constructed a proper parameterised PDO query. Hiring a dev might be the best way. – Apr 06 '14 at 14:54
@Tommie, yes, you don't need `mysql_real_escape_string`, although I've fixed the error that prompted that. – halfer Apr 06 '14 at 15:25
Oke guys, im getting this error after changing the code to this – Socket1 Apr 06 '14 at 15:33
@halfer i think its the magento/pdo which doesnt accept real escape string I will search the interweb for the right answer. Keep you in the loop. – Socket1 Apr 06 '14 at 17:49
$write = Mage::getSingleton("core/resource")->getConnection("core_write"); $query = "insert into mage_example (name, email, company, description, status, date) values (:name, :email, :company, :desc, 0, NOW())"; $binds = array( 'name' => "name' or 1=1", 'email' => "email", 'company' => "company", 'desc' => "desc", ); $write->query($query, $binds); @halfer this.. – Socket1 Apr 06 '14 at 18:27
Yep. Try modifying any queries that need parameters to that form, and if you get stuck, there should be some Magento/Mage docs. – halfer Apr 06 '14 at 18:43

Syntax error or access violation: 1064 ' brandname

2 Answers2