current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 1 down vote favorite

Looking at $wpdb class, I found get_row, and I am unsure if the function automatically prepares the query. I believe it does not and I may have to reexamine my code to prevent a future disaster.

Here is what my code looks, and I feel that it is vulnerable:

$wpdb->get_row("SELECT * FROM `my_table` WHERE column = '".$_GET['fromUrl']."'", ARRAY_A);

However, I have tried injecting a malicious input with the follow: ' or column = 'value'

And it seems not to work, should I go ahead and prepare my statements anyway?

share|improve this question
add comment

1 Answer 1

up vote 1 down vote accepted

Yes, you should prepare the statement or at least SQL escape the value of $_GET['fromUrl']. It's good practice to be paranoid about any input potentially provided by the user as it will be wrong at some point.

The wpdb-reference says:

Briefly, though, all data in SQL queries must be SQL-escaped before the SQL query is executed to prevent against SQL injection attacks.

The prepared query would look something like this 

$wpdb->get_row(
       $wpdb->prepare("SELECT * FROM `my_table` WHERE column = %s",
                      $_GET['fromUrl']),
       ARRAY_A);

which in my opinion is more readable, particularly the SQL query is easier to follow.

Furthermore, inspecting the user input may allow you provide the user better experience by creating meaningful notifications describing what has happened and how the user can cope with the inconvinience.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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