Quote from: dree12 on August 29, 2013, 12:35:07 PM

Review

A well-designed site, but there are many issues.

First the big stuff

CSRF vulnerability

There is a major CSRF^[1] vulnerability in various parts of your website. A malicious attacker can exploit this vulnerability to steal funds from bettors. Please PM me if you need any more information on this issue.

Most visible is the withdrawal script. An attacker can update the withdrawal destination and withdraw arbitrary amounts from user balance. A malicious webpage contains two inline frames, one of which updates the withdrawal address through a POST to http://ebitcoinbetting.com/account.php?id=2&a=1, and the other which withdraws money through another POST to http://ebitcoinbetting.com/account.php?id=2&a=2.

As far as I can tell, this isn't a problem with the password change form because the old password is required. This is also not likely a problem with betting, as there is a confirmation screen for that. Confirmations screens are generally poor at solving CSRF attacks, however, and care should be taken.

(On a related note, withdrawal does not currently work. The error given is:)

Quote

Catchable fatal error: Object of class mysqli_stmt could not be converted to string in /home3/dokula/public_html/ebit/account.php on line 186

^[1]: More information on CSRF and how to prevent it.

Some thoughts

Combined login/register

The combined login/register form is a bad idea. When I went to log in, I had to scroll down to see the Login button and initially thought I should fill my information in the upper form, which was instead for Registering.

Logo discrepancy

The logo, which says EBitCoingBetting, is confusing. The site's name is given elsewhere as EBitcoinBetting, without the g that follows the Bitcoin. Regardless of whether this is an error or an intentional discrepancy, it is a source of confusion. Brand should be unified, even if the difference is only a g.

Cosmetic balance display issue

On two separate occasions, the Balance in use shows nothing when no balance is being used. See: http://i.imgur.com/3bB0NsB.png and http://i.imgur.com/YX8jDN0.png.

Username issue?

On the deposit page, my username is displaying thus:

Quote

Hey, l3jNF!

The username I signed up with is not l3jNF. I'm not sure what this is. Perhaps this is intentional, but clarification would be helpful if that is the case.

Bet sizes

The minimum bets for all the ones I can see is 0.21 BTC. Not sure if this is intentional, again, but it certainly makes testing the bet system hard, since we only have 0.1 BTC to work with...

All of these have been fixed as well. Amazing write up! Thank you!