Add $id parameter to consist only of numbers before sending to Mysql

[amber]

I got a notice from my web host stating I needed to change some code on my member_detail page to be more secure: I pasted the email message from webhost  below & the code they say needs to change.  I have two questions regarding this: What code do I need to add/delete to make this change & are there any other steps besides the change that need to be made anywhere else?

 

 

Here is what the email said:

Earlier this morning, our security systems detected that someone logged into your WordPress site (using the WordPress "admin" username and
password) and installed malicious scripts that were designed to attack other sites. We also saw that the home page had been replaced with an obscene message.

We immediately locked the site and investigated what happened. We found that "hackers" took advantage of a security flaw in this file (if you're not the author of this file, you should forward this message to the
author):

 /wp-content/plugins/tourism-members/member_detail.php

That file contains the following code:

-------------------------------------------------------------------------------

 $id = $_GET["id"];

 global $wpdb;
 $query = 'select * from websiteirm WHERE id = ' . $id;  $result = $wpdb->get_row($query);

-------------------------------------------------------------------------------

This code allows an "SQL injection attack" where a visitor can send an "id" parameter &

This results in your eventual query to MySQL looking like:

 select * from websiteirm WHERE id = 999999.9 union all select user_activation_key from wp_users;

And that (in a more complicated version) allows the hackers to view the contents of the WordPress "user_activation_key" on the resulting Web page.

The "user_activation_key" is the secret code you get sent if you request a lost password from WordPress -- so what the hackers did is:

 1. Used the normal lost password form to request a "lost password" for
    the admin user;

 2. Viewed the secret code using the trick above;

 3. Used that code to reset the "admin" user's password; and

 4. Logged in to WordPress normally, giving them full rights to do anything.

Based on your site logs, it doesn't look like the hackers initially targeted your site specifically. They found this vulnerability by simply running automated software that searches for HTML parameters on any Web page, then sends specially crafted parameters to determine whether the parameters are susceptible to SQL injection problems.

In any case, since the hackers had complete control of the site after they did this at 11:04 AM Pacific time, the only thing we can do is restore it to the state it was in as of the last backup before it happened, made at 7:38:13 PM last night. We have done that, then disabled the vulnerable "member_detail.php" file by renaming it to:

 member_detail-DISABLED-FOR-SECURITY-TICKET-1559231.php

You'll obviously need to fix the problem in that file before making it publicly visible again. That's outside the scope of what we can assist with, but a Google search for "SQL injection" should help explain what you can do. (For example, in the case of the line above, the code could verify that the "$id" parameter consists only of numbers before sending it to MySQL.)

 

I know how to make changes to Filezilla etc but I need to know what exactly to put in this code to make it more secure, such as verifying the $id parameter consists only of numbers like my webhost suggested. Here is the code again.

-------------------------------------------------------------------------------

 $id = $_GET["id"];

 global $wpdb;
 $query = 'select * from websiteirm WHERE id = ' . $id;  $result = $wpdb->get_row($query);

-------------------------------------------------------------------------------

 

Then once those changes are made is there anything else I need to do besides changing this code?

[requinix]

First find the author of that plugin and see if there's an updated version.

 

Otherwise change it to

$query = 'select * from websiteirm WHERE id = ' . (int)$id;  $result = $wpdb->get_row($query);

[amber]

Will do. And this may be a stupid question but if I change the code to the one above then do I need to change something elsewhere? Want to make sure I don't get an error because I didn't add numbers to something else.

[requinix]

Considering how incredibly easy it is to prevent that kind of attack, and that the author didn't do it, there's a good chance there's more exploits to be found.

[amber]

I wondered about that because the search I did on SQL Injection said it was a known issue. Not sure about how that code was obtained because I took this over for someone after that was inserted. There wasn't an update for that plugin so I used your fix and it worked. Thank you!