-5

I have a simple website hosted on 000webhost that consists of index.html and register.php. I now have a functional registration system that allows users to input information, and have the database query the info.

On 000webhost, within public_html I have: php: register.php , .htaccess , index.html

index.html simply contains a button that links to the register page, and register.php contains:

<?php

session_start();

if (isset($_POST['register_btn'])){
    if ($_POST['email1'] != $_POST['email2']){
        exit("Emails did not match.");
    }

    //connect to database
    $connect = mysqli_connect("localhost", "someUser", "somepassword", "somedb");
    if(mysqli_connect_error()){
        echo mysqli_connect_error();
        exit();
    }

    //write the paramaterized query
    $query = "INSERT INTO test_table(username, email) VALUES(?, ?)";

    //prepare the statement
    $stmt = mysqli_prepare($connect, $query);

    if ($stmt){
        //bind
        mysqli_stmt_bind_param($stmt, 'ss', $_POST['username'], $_POST['email']);

        //execute the query
        mysqli_stmt_execute($stmt);

    } else{
        echo "Error creating statement object";
    }

}

?>

<!DOCTYPE html>
<html>
<head>
    <title>Register</title>
</head>
<body>
    <div class="header">
        <h1>Register</h1>
    </div>

    <form method="post" action="register.php">
        <table>
            <tr>
                <td>Username:</td>
                <td><input type="text" name="username" class="textInput"></td>
            </tr>
            <tr>
                <td>Email:</td>
                <td><input type="text" name="email1" class="textInput"></td>
            </tr>
            <tr>
                <td>Re-enter Email:</td>
                <td><input type="text" name="email2" class="textInput"></td>
            </tr>
            <tr>
                <td></td>
                <td><input type="submit" name="register_btn" value="Register"></td>
            </tr>
        </table>
    </form>
</body>
</html>

EDIT Changed the focus of the question from proper password registration to simple data entry.

Josh Evans
  • 567
  • 1
  • 5
  • 16
  • 3
    1. You need to learn more about how php works 2. You clearly haven't viewed your page soutce – John Conde May 05 '17 at 22:05
  • If you're committed to learning PHP, have a look at the various [development frameworks](http://codegeekz.com/best-php-frameworks-for-developers/) out there. Find one that suits your style and needs. You may find that there's already a module that does 80% of what you want and you can add that 20% as customizations. [Laravel](http://laravel.com/) is particularly beginner friendly and has a wealth of community code available to help you solve problems. Most of these have a directory organization structure that prevents casual snooping and stealing of credentials. – tadman May 05 '17 at 22:15
  • 2
    **WARNING**: When using `mysqli` you should be using [parameterized queries](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) and [`bind_param`](http://php.net/manual/en/mysqli-stmt.bind-param.php) to add user data to your query. **DO NOT** use manual escaping and string interpolation or concatenation to accomplish this because you will create severe [SQL injection bugs](http://bobby-tables.com/). Accidentally unescaped data is a serious risk. Using bound parameters is less verbose and easier to review to check you’re doing it properly. – tadman May 05 '17 at 22:15
  • 2
    @MattClark Yes and no. If you botch your server config, the server will *gladly* serve up `index.php` as raw source. This is why you need to be extra careful when deploying anything written in PHP. Most frameworks keep a very minimal public stub, the code there doesn't reveal anything useful, and hide all the actual code outside of the web root. – tadman May 05 '17 at 22:16
  • 1
    You should use `password_hash()` with `password_verify()` to properly hash your passwords. Then, anything you don't want public, put it outside of the public folder. Then include it where you need it. Although, that information is (should) be handled at server-side anyway, so you wouldn't see it when inspecting the source code (try it, you'll see). Also take note of what's been mentioned above about *prepared statements*. Escaping isn't enough. – Qirel May 05 '17 at 22:24
  • @MattClark MD5 is plain-text these days. It's a joke to crack. They should *not* be salted. [`password_hash`](https://secure.php.net/manual/en/function.password-hash.php) will do it all for you, no manual salting required. We've left the 1990s behind. If you're not using a password-strength hashing algorithm you're doing it wrong. – tadman May 05 '17 at 22:24
  • **WARNING**: Writing your own access control layer is not easy and there are many opportunities to get it severely wrong. Please, do not write your own authentication system when any modern [development framework](http://codegeekz.com/best-php-frameworks-for-developers/) like [Laravel](http://laravel.com/) comes with a robust [authentication system](https://laravel.com/docs/5.4/authentication) built-in. At the absolute least follow [recommended security best practices](http://www.phptherightway.com/#security) and **never store passwords with a uselessly weak hash like SHA1 or MD5**. – tadman May 05 '17 at 22:24
  • @tadman I was under the impression that mysqli_real_escape_string(Connection, String) prevented SQL injections by getting rid of special characters. But you're saying that this is not the case? – Josh Evans May 05 '17 at 22:27
  • Doing it manually is something that should be done only in exceptional circumstances. Notice how you still have variables in your query string that are interpolated. What if one of those wasn't escaped by accident? Then you have a SQL injection hole, and that's game-over for your site. Using placeholder values means you need to make *multiple* mistakes before you're exposed to that kind of risk, it's more secure by design. Remember, it takes one and only one mistake to open up your site to a world of hurt that can destroy everything. – tadman May 05 '17 at 22:28
  • 1
    Using prepared statements is easier than doing that on each value anyway, just bind them. But yes, there are some obscure cases where escaping isn't enough - http://stackoverflow.com/questions/5741187/ – Qirel May 05 '17 at 22:28
  • *"I worry that this is bad practice, as someone could "view page source" and see all of this information which I obviously don't want them to have access to."* - Which information is that? and access to what? – Funk Forty Niner May 05 '17 at 22:47

1 Answers1

1

There's a lot going wrong here, but I don't mean to blame you specifically. Many of the things that make PHP popular, like it's long history of support on shared hosting providers and how it's easy to pick up and get going, are also a huge liability: There's a lot of dangerously old advice out there, and many tutorials are written by people who really do not know what they're doing. It is a minefield to navigate, especially if you have never done this sort of thing before.

To side-step all of that you can use a development framework where modern PHP coding practices are on display. The best of these organize the code so that there's a very minimal amount of raw PHP code in the web root, that is the files that are accessible to the public. Many have just one stub, index.php, which everything is routed through. All the other URLs are, in essence, faked out by routing. There's no literal file to back them up. This is good because it decouples URLs from links to files and instead turns them into abstract resources.

This is how they manage to keep the application code from leaking out: The code put in the public web root is extremely minimal by design. If your server ends up misconfigured for whatever reason and starts spewing out .php files as raw source there's no harm, all they get is the boilerplate index.php stub that comes with every application of that type. No credentials, no database configuration information, nothing.

Laravel is a good example to study, it even includes an authentication system so there's no need to code your own. The database config in this case is just a regular PHP file, but other systems use JSON, YAML, or an old-fashioned INI file.

The biggest problem here is the use of string interpolation to compose your queries. If you make a mistake, forget to escape something, you'll open up a huge SQL injection hole. From there it's possible that someone can inject arbitrary SQL code into your site, and with that, grab the whole database or worse, maybe download the application's code as well if the database is running on the same physical machine.

At the absolute least use something like PDO. Even better, use an ORM that encapsulates your database operations in a much easier to use object-oriented wrapper. Doctrine, Propel and Eloquent are all good examples of these.

The most important thing to note is that making a secure site in 2017 is not easy if you're building from the ground up using just core PHP. There's so many things to consider: CSRF, XSS, SQL injection, password hashing, email verification, password guess rate limiting, caching, database migrations, and a host of other things. Fully understanding and implementing a coherent strategy for any one of those might months. It's beyond the scope of a single developer to realistically achieve, though I do encourage you to at least understand them on an academic level, the theory and implications of each.

PHP is lucky it has a lot of very good, well supported frameworks that come in a variety of forms. Find one you really like, learn it well, and use that for your projects. You'll be able to focus on writing code that makes your application unique instead of wasting a good chunk of that time badly re-implementing the wheel.

tadman
  • 208,517
  • 23
  • 234
  • 262
  • I've scrapped the escaping, added parameters, added binds, and used a better hashing algorithm. Any other red flags you notice? I understand the advice toward a PHP framework, but I'm more interested in learning at the lower level first. – Josh Evans May 06 '17 at 14:38
  • The reason PHP has a terrible reputation for security is because people make the mistake of assuming they can do it themselves. This stuff is really, *really* hard to get perfectly right, and anything less than perfect can be punished severely. Community code is safer because many people review it, and it's attacked constantly. Any little faults are quickly repaired and it's breaking news. If your code gets hacked you'll likely never know until it's too late to do anything meaningful. There's safety in numbers. – tadman May 06 '17 at 18:17
  • I understand that inclination, but there's better ways. Start with a framework and learn from it, dig your way down to the lower levels as you need to, not because you have to. All frameworks are open-source, you can always crack open the code and have a look to see how it functions, learning from good examples. Don't be afraid to explore and learn, but do have a healthy fear of not implementing your own *everything*. You need to develop that sense of when you need to roll your own, and when a canned solution will be more than adequate. – tadman May 06 '17 at 18:17
  • It's extremely rare to see a PHP project written without a framework that's understandable, maintainable, and coherent. Every time I've inherited one of these projects the first thing we do is salvage what we can and burn the rest to the ground. In the case of a project with a framework we'll at least consider supporting it after some due-diligence. A custom-everything project cannot and will not be trusted by anyone. – tadman May 06 '17 at 18:24