Mysidia Adoptables Review

[nobackseat]

I posted this in "virtual pet information" because users could pick this as their choice for a site base. I figure this review will be viewed more in this category too.

-------------------

So, on another forum, I was prompted to review the Mysidia Adoptables script, after saying it was awful. I figured I should finally back up my claims, so I am posting this. You make the judgement.

--------------

I downloaded the latest version of Mysidia Adoptables v1.2.2.

The first file I opened was login.php

This single excerpt of code from the login.php page tells a LOT about the quality of the code.

// Try to log the user in
$password = md5($password);
 
$query = "SELECT * FROM ".$prefix."users WHERE username = '$username'";
$result = @runquery($query);
$num = @mysql_numrows($result);
 
//Loop out code
$i=0;
while ($i < 1) {
 
$luser=@mysql_result($result,$i,"username");
$lpass=@mysql_result($result,$i,"password");
 
$i++;
} 

First, off they are using MD5()! MD5 is oudated; AKA twenty years old! And there are loads of websites that offer to "decrypt" almost any hash you can come up with.

Secondly, they use SELECT *, (grab the entire user row) just for checking if the row exists! What a waste of processing time, and memory!

Thirdly, they are using mysql_numrows! This function doesn't even exist on the PHP website itself. I'd say it's deprecated. The 'modern' alternative is mysql_num_rows, which is used in some places (it is pretty inconsistent throughout the script).

Lastly, anyone who has almost any kind of programming knowledge can recognize that the loop runs once. It is designed to run once. Does anybody else see what is wrong with that?! Why loop when it is only going to run once? The loop part shouldn't even exist.

------------

Also, I've noticed that they set their cookies as 'auser' and 'apass'. I don't think it could be any more obvious to specify what each is. Considering they are also using MD5, any XSS attack on the website, would not only let you replace the cookies to sign in as them, but also (most likely) get their actual password!

Additionally, they are using PHP Globals. I don't think I need to explain this. Google why PHP Globals is bad practice, insecure, and generally awful.

Look at these lines I pulled from functions.php. It should prove just how sloppy and unorganized it is.

    $GLOBALS['username'] = $username;
    $GLOBALS['loggedinname'] = $username; // MESSY - I'm unsure of which {username/loggedinname} is the correct one to use. 

Apparently some programmer didn't know what variable was what. So they are using global variables to set the same values twice, for nothing? More wasted memory.

The final thing I am going to mention is their habitual use of error suppression. I wrote about this in a blog post of mine on another site, but suppressing errors is very bad practice. Not only will the page show as blank, when an error exists, but sometimes the error lines in other places are thrown off. The interpreter 'gets confused' if I may, and can shoot out the wrong error line. What a debugging mess.

I see posts everywhere on the forum, where users are getting errors concerning their database credentials. The user doesn't understand that it is what it means.

They could simply add an 'or die('Your database information is wrong.')'.

Just think of how easy it is to add that on the connection query, and it would save so much time and energy.

Not to mention that errors give users and even hackers more information than they need to know. Should know.

Well, that is my conclusion of the script, simply from viewing login.php and about 1/3 of functions.php.

NBS

[kami]

That's pretty disgusting. A TON of people use those scripts that they offer, and they are all so easily open to attack? I'd be so ashamed if I wrote that >_>

[Gunmetal]

That's a frightening amount of problems you found from just two files! Hopefully they are corrected.

[cpvr]

Hopefully these problems are resolved soon.

[nobackseat]

I have spoken with the owner and some of the development team, and they have expressed genuine concern in the matter, and are working to improve as many of these problems for the next few releases.

I still would like to stress that the current version of the script is in disorder.

NBS

[nobackseat]

Unfortunately, it's been over 3 months since this and there is still no official release from the site repairing any of these issues.

I can assume that, as volunteers, they have priorities but they have a moderately sized user base that needs these updates.

I'm not sure if I should post on their forum again or not. Does anyone think it hasn't been enough time?

Thanks,

NBS

[Bessie11]

Hello Everyone,
what happened about your problem NBS.

[kami]

If they can put out updates at least once a month before, I don't see why this has taken three months with no updates. Even if it's just one fix at a time, that's better than nothing.

[Kesstryl]

Just an update, I'm a user of their softward, and it has gone through several versions since the last post and all security issues have been addressed and dealt with. I'd post a link to their current release 1.3.0 but I'm new and I'm not allowed to post links yet. If the OP would like to download the current version and look into the current security and find any other weak spots, I'm sure they would appreciate the feedback.

[Roconza]

Here is the new link http://www.mysidiaadoptables.com/for...ead.php?t=3495

It was just released so there may still be some bugs.

May I also say that Hall of famer's team has been working hard on this script and there has been many updates on the new script before it was posted. Maybe you guys should try being a active member on the forum before you harp on them. The team has been busy and the over haul needed to be pushed back a few times.

Really I don't like the amount of nativity at the people who are volunteers and have school to deal with.