SQL injection security question (Beginner Developer Course)

  • Jonathan Miles

    (@jmileswebdev)


    1 week, 6 days ago

    Hi there,

    I have been working with WordPress for a number of years, but to ensure I am following current best practice I am doing the Beginner WordPress Developer course on Learn.Wordpress.org.

    I have got to the Securely developing plugins and themes page (https://learn.wordpress.org/lesson/securely-developing-plugins-and-themes/).

    I have installed the sample plugin and got it working but am stuck on the Sanitizing inputs section:

    $name = $_POST['name'];
    $email = $_POST['email'];
    global $wpdb;
    $table_name = $wpdb->prefix . 'form_submissions';

    $sql = "INSERT INTO $table_name (name, email) VALUES ('$name', '$email')";
    $result = $wpdb->query($sql);

    As you can see the data is being saved directly to the database, without any sanitization.

    This means that if a user were to submit a name of John'; DROP TABLE form_submissions;-- the SQL INSERT query would be run, followed by the DROP query, and the table would be deleted from the database!

    For purely learning purposes on my dev site, I tried to entering a name of John’; DROP TABLE form_submissions;– into the name field of the form (put on a page using the plugin shortcode) and it seems some kind of sanitization is actually happening?

    The data saves as normal despite the SQL injection code. Has something changed to enhance the security?

    I also tried adding the table prefix (I’m using the default of ‘wp’):

    John'; DROP TABLE wp_form_submissions;--

    But this did not work either.

    Anyone got an answer to this? I obviously don’t strictly need to be able to break my own site (lol) but would be satisfying to know what the issue is here to aid my learning of what to avoid.

    When I run the SQL in the DB management tool AdminNeo, the query does do what is expected so it is very strange.

    I am running WordPress 7.0 on a local dev environment (LocalWP) using nginx and MySQL 8.0.35 if that’s helpful. Thanks in advance for your support!

    The page I need help with: [log in to see the link]

Viewing 3 replies - 1 through 3 (of 3 total)
  • Moderator threadi

    (@threadi)

    1 week, 6 days ago

    Take a close look at what kind of SQL statement would be generated if you entered this value in the “name” field. My recommendation would be to enter this value in the ‘email’ field instead for it to work. In the example shown there, “only” an SQL error would likely appear in the error log.

    Either way, validating user input is absolutely essential. This isn’t done automatically in the sample code or in the core functions used by this code. You’ll need at least two sanitize functions and $wpdb->prepare() to handle this properly.

    Thread Starter Jonathan Miles

    (@jmileswebdev)

    1 week, 5 days ago

    Thanks threadi for the quick response. I’ll have another look at this following your suggestions, cheers

    Thread Starter Jonathan Miles

    (@jmileswebdev)

    1 week, 4 days ago

    So I went down an interesting rabbit hole with this.

    It appears this particular vulnerability does not exist in modern WP versions, while using the functions in the example:

    1. WP has a magic quotes functionality that automatically escapes quotes in $_POST data (for historical compatability reasons, should not be relied upon for security today)
    2. If above protection is removed (eg you take out the slashes to manipulate the user submitted data in some way), $wpdb->query only allows one SQL query at a time

    Sources:
    https://developer.wordpress.org/reference/functions/wp_magic_quotes/
    https://wordpress.stackexchange.com/questions/21693/wordpress-and-magic-quotes
    https://wordpress.stackexchange.com/questions/304981/how-to-execute-mulitple-statement-sql-queries-using-wpdb-query

    So I got to the point where the malicious user input caused an SQL error, but could not drop the DB table. wpdb prevented any DB changes happening due to the multiple queries.

    The example code is still unsafe though
    But don’t worry I know the code example will be vulnerable in other ways, and is still bad code. Best practice exist for tried and tested reasons.

    I agree sanitize functions and $wpdb->prepare() should be used here to handle the user input securely.

    Just thought I would share what I found as may also be insightful for others about how WP works.

Viewing 3 replies - 1 through 3 (of 3 total)

You must be logged in to reply to this topic.