Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Security flaws #1

Closed
robincard opened this Issue · 13 comments

7 participants

@robincard

Hi BarrensZeppelin,

You have some big security flaws in your code, the flaw you have is the first exploit a hacker would try.

In competitions.php

$holdnavn = mysql_result(mysql_query("SELECT navn FROM teams WHERE id='". $_GET['id']."'"),0);

A hacker could run the following and you'd be in trouble

competitions.php?id=;drop table teams

@BarrensZeppelin

Hi robincard,

I appreciate your concern for the security of my project, but in this case there is nothing to worry about.

On the last lines of http://en.wikipedia.org/wiki/SQL_injection#Incorrectly_filtered_escape_characters :

While most SQL server implementations allow multiple statements to be executed with one call in this way, some SQL APIs such as PHP's mysql_query(); function do not allow this for security reasons.
This prevents attackers from injecting entirely separate queries, but doesn't stop them from modifying queries.

While I agree that it is good practice to check input data in SQL queries, php stops those kinds of injections per default.

@dbbk

Oh dear

@BarrensZeppelin

Well fuck. I guess it won't really prevent any injection but robincard's example..

1170690_709635819063405_439746313_n

@BarrensZeppelin BarrensZeppelin self-assigned this
@BarrensZeppelin BarrensZeppelin added the bug label
@robincard

Assuming someone did get access the unsalted md5 hash would also be a hackers dream "1a1dc91c907325c69271ddf0c944bc72" = 'test'.

It would be good to read up on a basic security guide.

@kleiram

It would be good to read up on a basic security guide.

This would be a great start! The entire "book" is worth a read by the way!

@BarrensZeppelin

I'll make sure to read it. ;)
I also uploaded a small commit which should fix the worst security issues.

We're here to learn, right? (:

@kleiram

You're doing better than a lot of other people. You don't stick your head in the sand and pretend it's fine so that's awesome.

@BarrensZeppelin

Doing my best.
6744922-pretty-young-girl-with-thumbs-up

@BeingTomGreen

@BarrensZeppelin - massive respect for actually listening and improving your code, too many people would have just played ignorant!

Is there any reason you chose to use the mysql_* functions rather than mysqli_, or better PDO?

The mysql_* functions are being deprecated in 5.5, this post, although now a little dated helped me massivly when converting from mysql_* to PDO.

@BarrensZeppelin

@BeingTomGreen Heh, thanks.
I'll definitely look into the link you've given me. I know that mysql_ will become deprecated, but I've just not been bothered to learn how to use mysqli_ instead.

My main interests lie in languages like C++ and Java, and I was taught PHP by a friend, so I've never really done a serious effort to become good at it. ^^

EDIT: On the other hand it should make it way easier to transition into coding in an object oriented style. ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.