Click Counter

Palgie

Hey, I'm working on a big project and it has a click counter / link exchange, kind of like what you see on a top site list, where it counts the hits that the website has gained.

Now i can figure out how to do this as I've not worked with mySQL(update) alot, however what i'm trying to gain is to be able to, when the file is loaded the field "out" in the database is increased by one every time, in accordance to "lid" (Link ID)

heres is the code, hopefully you can understand better than im explaining by looking at this.

//Adds to the click count for a particular link
db_query("UPDATE links SET out = out + 1 WHERE lid=".$_GET["out"]."","none");

However, i get the error.

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 'out = out + 1 WHERE lid=10' at line 1

Can anyone gimme a hand in pointing out where im going wrong, it would be appreciated greatly.

Thanks in advance,

Phil.

p.s the code is present in pass thru file, so when it goes to the file the codes present in, then it does what it needs to do then redirects, so there is very little connected to this.

bradgrafelman

As per the table shown here in the manual, "OUT" is a MySQL reserved word. Thus, you need to delimit it with backticks (`) so that MySQL knows that you're referring to a database/table/column/alias/etc. and NOT the special reserved word.

Also note that your code is vulnerable to SQL injection attacks. User-supplied data should never be placed directly into a SQL query. Instead, it should first be sanitized with a function such as [man]mysql_real_escape_string/man (although in your case, you could instead cast the $_GET["out"] variable to an int to ensure that non-integer data can't get into your query).

Palgie

Thank you very much, Your advice solved the problem i was having, however the lack of security is a worry as it is a script I'm coding, which was planned to sell in the next few weeks.

Is it against the forum rules to gain further contact to you for advice purposes such as MSN?

I've got around 30,000+ lines of code to go through to attempt to secure it fully with "mysql_real_escape_string()" and any advice would be a brilliant help!

All the best,

Phil.

leatherback

Phil: you could consider to include a little processing snippet on top of each script that parses your $GET array, and returns the variables back to the $GET array.

However.. If you fail to include such basic security precautions, you should not even consider selling the code.

Palgie

Hey, cheers for the reply.

Sorry for the faults, however I work in a small team and I stupidly didn't remember that we had security tightening already present to be done, However its been noted to be added at the end of the script, Im the graphics designer/coder, but my main area is graphics as you can see from my mistakes, and I will not be allowing the script to be released without as tight as security as we can, I guess ill need to read up on it abit further to assist.

Thanks for the $_GET suggestion also, much appreciated and it will be something worth looking at to the near future.

The security should have been done as we worked on the project, however I only started to become a tad MySQL friendly mid way into the project and haven't hadn't realised it wasn't done by the other coding staff.

Appreciate the help, All the best,

Phil.

bradgrafelman

leatherback wrote:
Phil: you could consider to include a little processing snippet on top of each script that parses your $GET array, and returns the variables back to the $GET array.

The problem with this is that of integrity: if you modify the $_GET array right from the start, you've lost the integrity of the data.

In other words, what if you later wanted to echo out say an article title they've submitted? You might very well end up with something like: "Thanks for submitting your article about M&M\'s!" - obviously not the desired effect.

In my opinion, nothing should be done on a global scale such as this; using [man]htmlentities/man for output sanitization or [man]mysql_real_escape_string/man for database sanitization should only occur at those respective layers - either output or database interaction.

@: This may be a bit late, but if you wanted a more robust approach to security, you could consider switching to prepared statements which, AFAIK, handle the proper escaping for you. Otherwise, you'll simply want to comb through each and every database query and examine the data that goes into that query; does this or that piece of data have the potential of being maliciously altered by a user? Note that the "user-supplied data" phrase isn't limited to $GET and $POST data; certainly $COOKIE and even $SERVER superglobals contain user-supplied data that can potentially be exploits in an application using them.