Review Desired: MySQL IP database

Any security holes (SQL injection) or improvement suggestions?

[JAB Creations]

This is my first PHP/MySQL script that I've written on mostly my own merits with the help of my buddy Greg. It's a very simple script, it adds a visitor's IP address to a MySQL database along with the current date. It's to be used in conjunction with other scripts later on. However on it's own merits I'd like to have folks take a look at this part specifically since it's my first time coding with MySQL. I've heard about MySQL injection attacks though I'm just not understanding exactly how it works other then a back or forward slash being inserted somehow?

I'm also interested in suggestions for improving the efficiency of the code in two ways, my if/else structure and in regards to how long I retain IP addresses. I know traffic levels make that specific topic subjective and I'm not wild about revealing specifics of my statistical analysis so a question I feel might help is what number of rows do I need to start becoming concerned about when MySQL reaches any upper limit in regards to it's auto-increment feature? Retaining an IP for a year versus a week might make sense if it's lower then having the same IP constantly create an incrementally high number?

I've taken care to add useful comments through out the code as much as possible. Thanks in advance for any and all useful replies!

- John

// Step #1&2: Connection to the database.

// Step #3: Create Table if none exists
mysql_query("
CREATE TABLE jab_ip_addresses (
id int(10) unsigned NOT NULL auto_increment,
date date NOT NULL,
ip varchar(20) NOT NULL,
PRIMARY KEY (id)
) ENGINE=MyISAM DEFAULT CHARSET=latin1 AUTO_INCREMENT=1;
");

// Step #4: Assign IP address in PHP to variable
$ipaddress = getenv("REMOTE_ADDR");
$date = Date("Y-m-d");

// Step #5 NEW--Determine if IP already exists and if so remove entry?
// Retrieve all the data from the "jab_ip_addresses" table
$result = mysql_query("SELECT * FROM jab_ip_addresses")
or die(mysql_error());

// store the record of the "jab_ip_addresses" table into $row
$row = mysql_fetch_array( $result );
echo '<br /><br />IP: '.$row['ip'].'<br />';
echo 'MySQL Date: '.$row['date'].'<br />';
echo 'PHP Date: ' . $date.'<br /><br />';

// If removal request does not exist proceed
// Step #6
if (isset($_GET['purge'])) {echo '<br /><br />You have requested to have your IP purged from the database.<br /><br />';
// Delete the client's IP, confirm when the IP is added but it's database ID auto-increments.
mysql_query("DELETE FROM jab_ip_addresses WHERE ip='$ipaddress'")
or die(mysql_error());

echo '<br /><br />Your IP address has been purged from the database.<br />
Would you like to <a href="test.php">add your IP address</a> back to the database?
<br />';
die();
}
// No? Then run the script as usual...
else {echo '<br /><br />You have not requested to have your IP purged from the database.<br />
Would you like to <a href="test.php?purge">purge your IP address</a> from the database?
<br /><br />';}

// Step #7: Attempt to insert IP Address in to table
// Step #7.1: Select the IP column and determine if the IP address already exists?
$sql = "select * from jab_ip_addresses where ip='" . $ipaddress . "'";
echo $sql;
$result = mysql_query($sql);

// Step #7.2: If the result = 1 IP is added, else if result is less then 1 don't add!
if (mysql_num_rows($result) >= 1) {
$error = "The IP address is already listed in the DB.";
echo '<br /><br />'.$error;
echo '<br />result = ' . $result;
}
else
{
$query = "INSERT INTO jab_ip_addresses (ip, date) VALUES ('$ipaddress', CURDATE())";
// $result = @mysql_query ($query);
$result = @mysql_query ($query) or die(mysql_error());
echo '<br /><br />The IP ' . $ipaddress . ' was added to the DB-array because it was not listed.';
echo '<br />result = ' . $result;
}

// Step #98: close database when finished!
mysql_close($dbh);
?>

[willybfriendly]

// Step #4: Assign IP address in PHP to variable
$ipaddress = getenv("REMOTE_ADDR");
$date = Date("Y-m-d");

// Step #5 NEW--Determine if IP already exists and if so remove entry?
// Retrieve all the data from the "jab_ip_addresses" table
$result = mysql_query("SELECT * FROM jab_ip_addresses")
or die(mysql_error());

Why pull everything on step #5?

How about

$q = "SELECT * FROM jab_ip_addresses WHERE 'ip' = $ipaddress";
$result=mysql_query($q);
if mysql_num_rows [us2.php.net]($result)
{
//do what you need with the data, which should be a single row, or certainly far less than an entire table!
}

This should also do most of step 7 for you too.

[JAB Creations]

Thanks Willy,

I'm having a little trouble reading from the database and once I figure that out I think I can tackle what you're talking about as echoing information has become a goal I can't figure out at the moment...

Let's say your IP is 127.0.0.1, you've already visited and the first date you did was saved to the database. I'm trying to have the following query merely echo out the date (regardless of the format) and instead I'm getting some 'Resource id #2' or '3'?

- John

// IP address of course...
$ipaddress = getenv("REMOTE_ADDR");

$q = "SELECT date FROM jab_ip_addresses WHERE ip='$ipaddress'";
$result=@mysql_query ($q) or die(mysql_error());
echo $result

[JAB Creations]

Never mind, I figured it out! ;)

$ipaddress = getenv("REMOTE_ADDR");
$q = "SELECT date FROM jab_ip_addresses WHERE ip='$ipaddress'";
$result=@mysql_query ($q) or die(mysql_error());
while ($row = mysql_fetch_assoc($result)) {echo $row["date"];}

I'll keep looking at your code, I have a vague concept of what you're talking about. No need to pull everything if you're looking for something specific. Too bad tutorial sites lack repetition in order to make pattern detection less aggravating.

- John

[trillianjedi]

I'm also interested in suggestions for improving the efficiency of the code in two ways, my if/else structure and in regards to how long I retain IP addresses.

I would also look at your MySQL storage efficiency. You're storing the IP as a 20 byte varchar, whereas it only actually needs to be a 4 byte int.

Have a look at the INET_ATON function within both PHP and MySQL. Here's some sample code I pulled out of a script for an example:-

$sql = "INSERT INTO db (ip) VALUES('inet_aton('".$IPAddress."'))";
$sql = "SELECT INET_NTOA(`ip`) AS ipadd FROM db";

IP's stored in this way should be as *unsigned* integers.

Worth building two functions, one using INET_ATON and one not. Then time the results. What you would do is first convert your IP Address string into a 4 byte int, then do the MySQL search. I think you'll find a decent performance benefit.

The other point is that you have an index for the id field, but not the IP address field. As you're performing searches against the IP field you'll get a performance benefit from have that column indexed.

any upper limit in regards to it's auto-increment feature?

That relates to the type of variable you've declared to store the auto-increment field. You've chosen an unsigned int, which is basically good up to about 4 billion (or to be precise 4,294,967,295).

That's your upper limit. If you ever hit it, can I borrow your private jet from time to time?

;)

[JAB Creations]

I changed this...

ip varchar(20) NOT NULL,

...to this...

ip INT(4) NOT NULL,

...and only the first four numbers were added? An IPv4 IP address requires at least 12 bytes if you removed the periods but then again you could mix up 12.70.0.1 and 127.0.0.1. I obviously need clarification please, I know an integer is a number, I don't know what unsigned means, and how all of what you're explaining to me is supposed to work. Essentially that is why I echo stuff so much, so I know what a variable or command looks like at each step.

Can I just change the index from the id to the IP field to have the column indexed?

Help me figure this out and I'll let you take the jet to max cruising speed. :D Thanks for your input thus far!

- John

[RonPK]

Sometimes the manual [dev.mysql.com] actually clarifies things ... :)

mysql> SELECT INET_ATON('209.207.224.40'); 
 -> 3520061480 
   
The generated number is always in network byte order. For the example just shown, the number is calculated as 209×2563 + 207×2562 + 224×256 + 40.  

So your field will need to be longer than just 4 characters.

Btw, PHP offers similar (but not equivalent) functionality:

ip2long() [php.net]

and

long2ip()

.

[JAB Creations]

Let me make a simple guess...

Would an ip of 127.0.0.1 be stored as '127000000001'? It wouldn't seem to difficult to figure out where class divides belong then! ;)

- John

[RonPK]

Sorry, I forgot to superscript certain characters in my previous post. Your example IP address would be converted into a unique number like this:

127×2563 + 0×2562 + 0×256 + 1

[trillianjedi]

RonPK clarifies your questions (and my mistake on storage "space") I think, but just to explain further so that you understand how it's working...

An IPv4 IP address requires at least 12 bytes

You're thinking in terms of characters and not numbers. A single byte can store an integer value from 0 -> 255. Unsigned means there are no negative values.

0.0.0.0 to 255.255.255.255 (complete range of all IPv4 IP addresses) can therefore be represented in 4 bytes:-

Byte 1 : 0 -> 255
Byte 2 : 0 -> 255
Byte 3 : 0 -> 255
Byte 4 : 0 -> 255

If you look at that as an integer, you get a total number of 0 to 4 billion odd if you use RonPK's formula above. That's the way that integers work. The byte numbers are not joined back to back to make up an integer. Each byte represents a number of multipliers.

The INET_NTOA function exists to decode that back into the 4 individual bytes of an IP address. From there it adds the dots and turns it into a string.

The INET_ATON function works the opposite way around, taking a string and returning the unsigned 4 byte int.

According to my MySQL table the actual storage space required is 11 bytes for an unsigned integer in a MySQL table.

To extend Rons code and show this in both directions:-

mysql> SELECT INET_ATON('209.207.224.40');
+-----------------------------+
¦ INET_ATON('209.207.224.40') ¦
+-----------------------------+
¦   3520061480 ¦
+-----------------------------+
1 row in set (0.00 sec)

mysql> SELECT INET_NTOA(3520061480);
+-----------------------+
¦ INET_NTOA(3520061480) ¦
+-----------------------+
¦ 209.207.224.40 ¦
+-----------------------+
1 row in set (0.00 sec)

[Dabrowski]

ok, I've only had a quick skim through, but you're worried about the upper limit on AUTO_INCREMENT when you're using and int unsigned to store it? That means a limit of 4.2bn entries. I don't think you'll hit that anytime soon.

Second, lets talk binary with these IP addresses......

The 4 octets of your IP address are 8 bit numbers, an int unsigned in MySQL is a 32 bit number. See the correlation there?

So, take the IP 123.245.167.9, in binary this is:

123......245......167......9
01111011 11110101 10100111 00001001

But if you stuff those bits into an int as one number....
01111011111101011010011100001001 = 2079696649.

Remember all we're worried about here is the bits, we have the same bits represented in a different way. If you take those bits and split them into 4 chunks of 8 bits you will get your original IP address back.

[JAB Creations]

Part of the following is what I get when I export the table...

INSERT INTO `jab_ip_addresses` VALUES(1, '2008-02-15', 0);

Using...

$query = "INSERT INTO jab_ip_addresses (ip, date) VALUES('inet_aton('".$IPAddress."')', CURDATE())";

Those are the only two things I've changed. Though to confirm it do I have to use the inet_aton to convert the IP I'm using and then seek a match in the database? When I'm looking at the MySQL export should I expect to see my IP address as besides four numbers? I take it my IP != 0! Thanks for all the input from everyone! :)

- John

[trillianjedi]

What is the type of field you've set for jab_ip_addresses ?

[whoisgregg]

If you're going to use a MySQL function during your insert, it can't be inside of quotes, else it is interpreted as a string... Inserting a string into an INT isn't going to work.

incorrect

 VALUES('inet_aton('".$IPAddress."')',

correct

 VALUES(INET_ATON('".$IPAddress."'),

[JAB Creations]

I removed the quotes that would create a string.

I get the following error...

Column 'ip' cannot be null

But when the table is create ip is explicitly set to not null and when browsing PhpMyAdmin it's displayed as not null.

Edit: Trillian: three, 'id', 'date', and 'ip'.

[edited by: JAB_Creations at 6:56 am (utc) on Feb. 16, 2008]

[trillianjedi]

No - the field type for IP.

It should be:-

Type : INT
Length : 11
Attributes : Unsigned

[JAB Creations]

I changed it to...

ip int(11) UNSIGNED NOT NULL,

I still had the same error message so I changed it to...

ip int(11) UNSIGNED,

Though the value showed as being 'null'.

So I still have not been able to see anything that looks like what everyone has been describing in the thread?

- John

[trillianjedi]

It's better left as "NOT NULL".

Can we take out the $vars and try hard-coding something as a test. Maybe just create a new test script. Use this to write an entry:-

INSERT INTO jab_ip_addresses (ip, date) VALUES(INET_ATON('127.0.0.1'), CURDATE());

... and then read it with:-

SELECT INET_NTOA(`ip`) FROM jab_ip_addresses;

... and let us know what results you get.

[JAB Creations]

Keeping it as simple as I can...

<?php
// Step #1 connection related...
// Step #2: Choose the database
$selected = mysql_select_db("jabcreat_forums",$dbh) or die("Could not select first_test");

// Step #3: Create Table if none exists
mysql_query("
CREATE TABLE jab_ip_addresses (
id int(10) unsigned NOT NULL auto_increment,
date date NOT NULL,
ip int(11) UNSIGNED NOT NULL,
PRIMARY KEY (id)
) ENGINE=MyISAM DEFAULT CHARSET=latin1 AUTO_INCREMENT=1;
");

// Step #4
mysql_query("INSERT INTO jab_ip_addresses (ip, date) VALUES(INET_ATON('127.0.0.1'), CURDATE());")
or die(mysql_error());

// Step #5
$q = "SELECT INET_NTOA(`ip`) FROM jab_ip_addresses;";
$result=@mysql_query ($q) or die(mysql_error());
while ($row = mysql_fetch_assoc($result))
{
echo $row["date"];
echo $row["ip"];
}
?>

Once I drop the existing table and execute this file via the browser I went to PhpMyAdmin to export whatever entries might exist...

PhpMyAdmin Export

INSERT INTO `jab_ip_addresses` VALUES(1, '2008-02-16', 2130706433);

It looks like something! I'm not sure where to go from here though, nothing echoed from the PHP?

- John

[trillianjedi]

Try replacing Step #5 with this:-

// Step #5
$q = "SELECT INET_NTOA(`ip`) FROM jab_ip_addresses;";
$result = mysql_query ($q) or die(mysql_error());
while ($row = mysql_fetch_row($result))
{
echo $row[1];
echo $row[2];
}

[JAB Creations]

Using the revised step #5 PHP echoes nothing still.

[JAB Creations]

If I add echo $result;...

{
echo $row[1];
echo $row[2];
echo $result;
}

I get the "Resource id #3" for every single row (times I've reloaded the page since I last dropped the table and restarted). Does that help any?

- John

[willybfriendly]

$q = "SELECT INET_NTOA(`ip`) FROM jab_ip_addresses;";

I don't think that semi befor the close quote is supposed to be there.

What happens with:

$q = "SELECT INET_NTOA(`ip`) FROM jab_ip_addresses";

When queries are giving odd results I often start in phpMyAdmin. Run the query and see if it gives the expected result. If it does, then the problem is elsewhere.

[JAB Creations]

Removing it had no effect. I'm reading through a book and researching online. I'm not making any connections here though there aren't any clear examples and the book seems to lack INET_NTOA and INET_ATON. Does any one have any suggested reading for me?

[trillianjedi]

Does any one have any suggested reading for me?

Not for this particular problem, but in general I highly recommend Jeremy Zawodny - High Performance MySQL.

Back to problem in hand - do you have shell access to the server?

[JAB Creations]

I do have shell access though I only know a very limited number of commands. Just let me know what I need to do and I'll do my best. If there are any words I can reference in a search to find other people who have dealt with the problem please let me know, maybe they used different IP addresses? It seems rather strange that not much comes up for MySQL IP addresses when I search.

- John

[willybfriendly]

What are you editing in?

$q = "SELECT INET_NTOA(`ip`) FROM jab_ip_addresses";

` != '

Do you see what I am talking about in the above line? Those are not single quotes.

[trillianjedi]

I do have shell access though I only know a very limited number of commands. Just let me know what I need to do and I'll do my best

mysql -u <username> -p<password> -D <database>

SELECT INET_ATON('209.207.224.40');

See if we can get that far.

Those are not single quotes.

WBF - they're called backticks. They're not meant to be single quotes (it's a DB field, not a string). The syntax is correct.

[JAB Creations]

More fun...errors 1044 and 1045. I'm using the correct passwords. I presume this is only to connect to the database via shell?

- John

[JAB Creations]

aI think I should ask for clarification because when I try different stuff out I get the same results which means whatever *nix OS my host is running it's not giving me any helpful error messages.

Since I'm on a shared host my info looks like this...
User: jabcreat_test
Database: jabcreat_test

Would I at any point merely enter test versus jabcreat_test for either the user/database names? I just want to ensure the problem isn't that simple. I presume I should doing the same thing as how PHP connects to the database.