I'd like to vote on this as well instead of having  to 'Me too!' but I can't vote :-)

query INSERT INTO bugdb_votes (bug,ip,score,reproduced,tried,sameos,samever) VALUES(20720,,1,1,1,0,0); failed: 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 '1,1,1,0,0)' at line 1

The offending code would be:

$ip = ip2long($HTTP_X_FORWARDED_FOR ? $HTTP_X_FORWARDED_FOR : $REMOTE_ADDR);

Normally a single proxy sits between the client and the server, giving a single IP address that ip2long() accepts, but if a connection is bounced through a chain of proxies, the X-Forwarded-For header will contain a list of those IPs in the form of: "X-Forwarded-For: <ip>[, <ip2>, ...]". eg. "X-Forwarded-For: 10.0.0.1, 10.0.0.2". This is probably why ip2long() is failing for these two people.

Also, if you're not using a proxy that sets X-Forwarded-For, any client can set this header, making it untrustworthy. I could set my X-Forwarded-For: header to 'BLAH' and this script would fail.

I suggest someone with php-bugs-web karma simply remove the X-Forwarded-For part and change the offending line to:

$ip = ip2long($REMOTE_ADDR);

If it becomes a problem with multiple people behind the same proxy, then you can add in exceptions for those people as the issue arises.

Looking into this atm.

I'll commit the following patch later today if there are no problems with the suggested patch.  It's based on a patch from phpweb.

--jm

cvs diff -u include/functions.inc vote.php
Index: include/functions.inc
===================================================================
RCS file: /repository/php-bugs-web/include/functions.inc,v
retrieving revision 1.127
diff -u -r1.127 functions.inc
--- include/functions.inc       13 Jul 2004 21:51:25 -0000      1.127
+++ include/functions.inc       4 Aug 2004 12:04:10 -0000
@@ -578,4 +578,35 @@
        return array(" AND MATCH (bugdb.email,sdesc,ldesc) AGAINST ('" . addslashes($search) . "')", $ignored);
 }

+/* Figure out which IP the user is coming from avoiding RFC 1918 space */
+function get_real_ip () {
+       $ip = false;
+
+       /**
+        * User is behind a proxy and check that we discard RFC1918 IP
+        * addresses if they are behind a proxy then only figure out which
+        * IP belongs to the user.  Might not  need any more hacking if
+        * there is a squid reverse proxy infront of apache.
+        */
+       if (!empty($HTTP_X_FORWARDED_FOR)) {
+               $ips = explode (", ", $HTTP_X_FORWARDED_FOR);
+               if ($ip) { array_unshift($ips, $ip); $ip = false; }
+               for ($i = 0; $i < count($ips); $i++) {
+                       /**
+                        * Skip RFC 1918 IP's 10.0.0.0/8, 172.16.0.0/12 and
+                        * 192.168.0.0/16 -- jim kill me later with my regexp pattern
+                        * below.
+                        */
+                       if (!eregi ("^(10|172\.16|192\.168)\.", $ips[$i])) {
+                               $ip = $ips[$i];
+                               break;
+                       }
+               }
+       }
+
+       /**
+        * Return with the found IP or the remote address
+        */
+       return ($ip ? $ip : $REMOTE_ADDR);
+}
 ?>
Index: vote.php
===================================================================
RCS file: /repository/php-bugs-web/vote.php,v
retrieving revision 1.9
diff -u -r1.9 vote.php
--- vote.php    23 Jan 2004 03:05:28 -0000      1.9
+++ vote.php    4 Aug 2004 12:04:10 -0000
@@ -21,7 +21,7 @@
        or die("Unable to connect to SQL server.");
 @mysql_select_db("php3");

-$ip = ip2long($HTTP_X_FORWARDED_FOR ? $HTTP_X_FORWARDED_FOR : $REMOTE_ADDR);
+$ip = ip2long(get_real_ip());
 // TODO: check if ip address has been banned. hopefully this will
 //       never need to be implemented.

Make sure to test it carefully!

I have a feeling this still isn't going to work for me.

As I said, $_SERVER['HTTP_X_FORWARED_FOR"] is set to the 
string "unknown"
No IP numbers anywhere at all in it, or any kind of a 
number.

This is a common configuration option for the Squid 
proxy.

OK then it still needs to be tested whether the found IP matches an IP address regexp.

Okay I've slightly changed the patch.  Which uses ip2long on the current IP it's working through to validate if the string is really an ip address (http://php.mirrors.powertrip.co.za/ip2long) if we get -1 back from ip2long we know this a valid ip address.

if (!eregi ("^(10|172\.16|192\.168)\.", $ips[$i])) {
    if (ip2long($ips[$i]) != -1) {
      $ip = $ips[$i];
      break;
   }
}

My pseudo code test returns the expected ip address even when I use 'unknown' and various other varients for the forwarded address.

I've updated the patchfile and uploaded it to http://www.powertrip.co.za/patches/php-bugs-web-29151.diff for review.

> if (ip2long($ips[$i]) != -1) {

Remember, as of PHP 5.0.0 ip2long() returns FALSE instead of -1.

Dave thanks for the 'headsup'.  I have a patch against phpweb for the same issue as well as checking which version of php is running on the webserver. For phpweb look at this patch http://www.powertrip.co.za/patches/phpweb-29151.diff and I'll update the patch for php-bugs-web shorlty which will be made available over at http://www.powertrip.co.za/patches/php-bugs-web-29151.diff for review.  I've tested the phpweb patch on http://php.mirrors.powertrip.co.za/ and it works when going through a test squid cache server here at my office and then via two netcache boxes at SAIX.

I'll commit the following patch http://www.powertrip.co.za/patches/php-bugs-web-29151.diff in the morning as it works as expected on my development version of php-bugs-web.

This bug has been fixed in CVS. Since the websites are not directly
updated from the CVS server, the fix might need some time to spread
across the globe to all mirror sites, including PHP.net itself.

Thank you for the report, and for helping us make PHP.net better.