Mysidia Adoptables Review

[cpvr]

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.

They sure have come along way!

[Corzeir]

They sure have come along way!

They have worked hard on the script and they kinda got it with all the problems of the old script when the original coder sold it.. I think they have done very well expanding and fixing it.

[cpvr]

They have worked hard on the script and they kinda got it with all the problems of the old script when the original coder sold it.. I think they have done very well expanding and fixing it.

Yup, and hopefully someone can re-review the script and see if everything is good. @nobackseat

[nobackseat]

@cpvr,

The script, through all its versions, has been consistently ludicrous to the point where it's amusing.

My time only allots for a few files, but I'll see what I can find.

It seems the registration page seems to top all of the script concerning security.

<input name='answer' type='hidden' id='answer' value='{$answer}'></p>
<input name='ip' type='hidden' id='ip' value='{$_SERVER['REMOTE_ADDR']}'></p>

That is an excerpt from the registration form. It stores the answer to the security question in a 'hidden' input field, as well as the user's IP address, in its own 'hidden' input field.

Just because the browser chooses not to show it, doesn't mean that it's safe in any way. I can view the source of the page easily, and so can automated spamming software.

This means that I can edit the IP sent to the server, and that automated spamming software can grab the security answer from the input field and use it to register an account in an automated fashion...essentially defeating the purpose of what they call a 'Security Question.' Lol. Oh, by the way, the security question is the same for everyone who registers. So actually, there's no need for it to grab the input field, it can simply be hard coded into the 'bot'.

In addition, there is no real limit on the length of the username. There is a limit in the HTML, but that can be bypassed very easily. And there is no check on the birthdate at all. No check on if it's even a real date (Feb 30?) or even numbers. Lol. You can type whatever you want in the field. Extra profile room anyone?

The majority of issues I have with Mysidia Adoptables continue to be efficiency and memory usage.

Such as comparing the initially entered password with the 'confirmed' password, after each one has already been encrypted (and the encryption used doesn't make much sense and is pretty sloppy). And grabbing all the data from rows unnecessarily (such as when checking if a username exists...). And the frequent use of curly brackets around variables was meant for certain conditions when using arrays, instead they seem to use it for cosmetic reasons.

Mysidia combines cookies and sessions, which is a good idea, except they seem to use it just to generate a unique ID for the cookie. Really just a waste.

There's just signs of beginners coding it everywhere. The oddities are small, but they are E-VER-Y-WHERE, which makes it inefficient and annoying to study.

$article_content = $regsuccess."".$username."".$regsuccess2;

You only need a single dot between variables. No quotes, and not the other period.

$pass1 = $_POST["pass1"];
$pass1 = secure($pass1);
$pass2 = $_POST["pass2"];
$pass2 = secure($pass2);

Uhhhh OK. Could just wrap the function around the array.

$salt = $_POST["salt"];
$salt = secure($salt);

It would be really funny if the salt really were stored in the login form. Alas, it's not so this code is... pointless. Actually, it uses up memory on your server, so at least it does that.

function passencr($username, $password, $salt){
    $pepper = grabanysetting("peppercode");
    $password = md5($password);
    $newpassword = sha1($username.$password);
    $finalpassword = hash('sha512', $pepper.$newpassword.$salt);
    return $finalpassword;
}

What a mess. Guys, are ya sure that's the final password...?

function __autoload($name) {
  // The autoload function, a bit messy if you ask me
  $classpath = strtolower("classes/class_{$name}");
  if(defined("SUBDIR")) include_once ("../{$classpath}.php");
  else include_once ("{$classpath}.php");
}

Who asked you? JK, totally agree. Now do something about it. Again, more showy brackets. Ooooo, aaaaah.

I'm having fun with this, but I ought to go back to work.

Before I forget, it still uses globals too, a point from my previous analysis. Globals are bad and messy!

NBS