0

I have a switch statement which determines the filetype of an image uploaded for use as an avatar in my application, however it seems to be a little faulty, insofar as it allows for a successful registration regardless of whether an allowed filetype is present or not, and no error messages are being returned re. the filetype submitted not being allowed.

$submit = $_POST['submit'];

if ($submit == 'Sign up!') {
    require_once("db_connect.php");
    $submit = clean_string($_POST['submit']);
    $first_name = clean_string($_POST['first-name']);
    $last_name = clean_string($_POST['last-name']);
    $email = clean_string($_POST['email']);
    $password = clean_string($_POST['password']);
    $confirm_password = clean_string($_POST['confirm-password']);

    //Output variables
    $register_bad_message = '';
    $register_good_message = '';

    require_once($_SERVER['DOCUMENT_ROOT'] . '/recaptcha/recaptchalib.php');
    $privatekey = "6Ldbd8ASAAAAAFz8VT29H5w4WLNjsbI-mFY2QkaC";
    $resp = recaptcha_check_answer ($privatekey,
                                    $_SERVER["REMOTE_ADDR"],
                                    $_POST["recaptcha_challenge_field"],
                                    $_POST["recaptcha_response_field"]);
    if (!$resp->is_valid) {
        $errMessage = $resp->error;
        $register_bad_message = '<div class="alert alert-error">The reCAPTCHA you entered wasn\'t correct. Please try again.</div>';?>
        <script>
            $('a.account-register').trigger('click');
        </script><?php
    } else {
        if ($first_name&&$last_name&&$email&&$password&&$confirm_password) {
            if ($password == $confirm_password) {
                if (strlen($password) > 25 || strlen($password) < 6) {
                    $register_bad_message = '<div class="alert alert-error">Please enter a password between 6 and 25 characters.</div>';?>
                    <script>
                        $('a.account-register').trigger('click');
                    </script><?php
                } else {
                    if($db_server) {
                        $first_name = clean_string($first_name);
                        $last_name = clean_string($last_name);
                        $email = clean_string($email);
                        $password = clean_string($password);
                        mysql_select_db($db_database);

                        $taken = mysql_query("SELECT email FROM users WHERE email='$email'");
                        $count = mysql_num_rows($taken);
                        if ($count > 0) {
                            $register_bad_message = '<div class="alert alert-error">The email you have entered is already associated with a Screening account. Please choose another.</div>';?>
                            <script>
                                $('a.account-register').trigger('click');
                            </script><?php
                        } else {
                            if ($_FILES) {
                                //Put file properties into variables
                                $file_name = $_FILES['profile-image']['name'];
                                $file_size = $_FILES['profile-image']['size'];
                                $file_tmp_name = $_FILES['profile-image']['tmp_name'];


                                //Determine filetype
                                switch ($_FILES['profile-image']['type']) {
                                    case 'image/jpeg': $ext = "jpg"; break;
                                    case 'image/png': $ext = "png"; break;
                                    default: $ext = ''; break;
                                }

                                if ($ext) {
                                    //Check filesize
                                    if ($file_size < 5242880) {
                                        //Process file - resize, clean up filename and move to safe location
                                        $image = new SimpleImage();
                                        $image->load($file_tmp_name);
                                        $image->resizeToWidth(250);
                                        $image->save($file_tmp_name);


                                        $n = "$file_name";
                                        $n = ereg_replace("[^A-Za-z0-9.]", "", $n);
                                        $n = strtolower($n);
                                        $n = "avatars/$n";
                                        move_uploaded_file($file_tmp_name, $n);
                                    } else {
                                        $register_bad_message = '<div class="alert alert-error">Please ensure your chosen file is less than 5MB.</div>';?>
                                        <script>
                                            $('a.account-register').trigger('click');
                                        </script><?php
                                    }
                                } else if (!empty($ext)) {
                                    $register_bad_message = '<div class="alert alert-error">Please ensure your image is of filetype .jpg or.png.</div>';?>
                                    <script>
                                        $('a.account-register').trigger('click');
                                    </script><?php
                                }
                            }
                            $password = md5($password);
                            $query = "INSERT INTO users (first_name, last_name, email, password, image) VALUES ('$first_name', '$last_name', '$email', '$password', '$n')";
                            mysql_query($query) or die("Insert failed. " . mysql_error() . "<br />" . $query);
                            $register_good_message = '<div class="alert alert-success">Registration successful!</div>';?>
                            <script>
                                $('a.account-register').trigger('click');
                            </script><?php
                        }
                    } else {
                        $register_bad_message = '<div class="alert alert-error">Error: could not connect to the database.</div>';?>
                        <script>
                            $('a.account-register').trigger('click');
                        </script><?php
                    }
                    require_once("db_close.php");
                }
            } else {
                $register_bad_message = '<div class="alert alert-error">Passwords failed to match. Please try again.</div>';?>
                <script>
                    $('a.account-register').trigger('click');
                </script><?php
            }
        } else {
            $register_bad_message = '<div class="alert alert-error">Please fill in all fields before continuing.</div>';?>
            <script>
                $('a.account-register').trigger('click');
            </script><?php
        }
    }
}

For example, uploading a .GIF file results in no errors and a 'Registration successful' message, however when logging into the profile, the uploaded profile photo is not shown. I'm thinking that the code is refusing the filetype and not storing it in the database, but is still processing the registration, rather than cancelling it, which is what it should do.

Alex Ryans
  • 1,865
  • 5
  • 23
  • 31

2 Answers2

1

You would have to set $ext to false and not '' because this isn't false for the if statement.

default: $ext = false; break;

Or you check if $ext isn't an empty string:

if ($ext != '') {

To prevent the registration when an invalid filetype is uploaded you have to put

$password = md5($password);
$query = "INSERT INTO users (first_name, last_name, email, password, image) VALUES ('$first_name', '$last_name', '$email', '$password', '$n')";
mysql_query($query) or die("Insert failed. " . mysql_error() . "<br />" . $query);
$register_good_message = '<div class="alert alert-success">Registration successful!</div>';?>
<script>
$('a.account-register').trigger('click');
</script><?php

Inside of if($ext != '') { /*Put code at the end of if*/} or if($ext) { /*Put code at the end of if*/ }. Otherwise it doesn't matter if there is a valid filetype.

Marcel Gwerder
  • 8,353
  • 5
  • 35
  • 60
  • Hi, thanks for your help! Unfortunately, neither of your solutions seem to be working. The application is submitting the registration successfully, apart from the avatar uploaded, so the check is apparently working fine, but the registration should not proceed if an invalid filetype is uploaded. – Alex Ryans Mar 31 '13 at 11:38
0

Sometimes, the content of $_FILES['profile-image']['type'] is not set up. For example, if you submit file from cURL or socket. I would try load mime type for myself from $_FILES['profile-image']['tmp_name']

EDIT:

I also noticed this construction:

  if ($_FILES) { .... }

There should be better to use

  if (isset($_FILES[key])){ .... }
Martin Perry
  • 9,232
  • 8
  • 46
  • 114