DEV Community: Jannis Mattheis

Review of my first Project

Jannis Mattheis — Sun, 19 Apr 2020 12:02:40 +0000

Originally posted on my website: jmattheis.de

Intro

In 2012 I (13 years old) started learing coding. I didn't read books or anything, I mostly copied stuff from YouTube tutorials. This lead to my first bigger project. A social network clone, which should be similar to facebook.

In this blog post, I'll review my now 8 year old code. Partly to amuse myself what kind of errors I made and to maybe learn something from it.

The project has the following features:

post timeline with like and comment functionality
"real-time" updates of comments and like counts for posts
chat between two users
friend system
user search
profile page
profile picture upload
login and register with email confirmation

It looks like this:

Yes, most of the stuff is in german, but I'll rename variables and strings in code examples to english.

Code Style/Quality

Style guides define rules on how code should look like. This is mostly done to enforce consistency, maintainability and reduce common programming errors. For PHP there are code styling guidelines defined in
PSR-1 and PSR-12.

My project didn't follow any of such guidelines and it sure looks like garbage. Bad code is the only consistency in that project :). In newer projects I use linters like eslint and formatters like prettier to ensure code quality and style.

Example of ugly code.

$number = 2;
$query = mysql_query("SELECT * FROM post_comment WHERE post_id='".$post_id."' ORDER BY comment DESC");
    WHILE ($row = mysql_fetch_assoc($query)){
    if($zahl != 0) {
    $comment    = $row["comment"];
    $name       = $row["name"];
    $date       = $row["date"];
         if(strlen($comment)>=47)
        {
            $comment = substr($comment, 0, 47)."...";
        }
        $end .= "<hr><a href='/profile/".$name."'><b>"    .user_return($name)."</b></a><b>|</b> ".$comment;
        $number = $number - 1;
        }
    }
    echo $end;

Unused Code

Having less code greatly increases code maintainability, code complexity, reduces potential bugs, and overall simplicity of a project. New team members not only have to understand used code but also the unused code which doesn't add any benefit and is only a waste of time.

Without code, it is possible to write a secure and reliable application ->
github.com/kelseyhightower/nocode

When parts of an application will be refactored. It is likely, that code will lose its callers and be unused. That's okay as long as the code doesn't get into production. Refactorings should be done inside a branch inside version control and be reviewed for code quality before going into production.

Reuse Code

Similar code should not be copied without reasoning. Depending on the context, it maybe perfectly fine to copy things. F.ex. in tests I prefer having all the relevant test setup inside the actual test method and not extracting it to another method.

In my project, there is alot copied code. I think the main reason was, that I really rarely used return in functions and rather just echo'ed/printed them out.

A prime example for code that should be reused is the rendering of posts inside the timeline. In there I created two functions one for the timeline for the current user, and one for the profile page of a user. The two functions are like 80 chars each and are pretty much the same, only the SQL query for getting posts is a little different.

File Structure

Structuring a project isn't easy. While I extracted code to different files, I didn't really grouped it well. There is a file in the project called action/functions.php, and like the name tells, it contains nearly all business logic. Every function in that file, has a header. It looks like this:

///////////////////////////////////////////
///////////////////////////////////////////
///////////////See All Posts///////////////
///////////////////////////////////////////
///////////////////////////////////////////
function See_all_post($id) {
    // content
}

I think I knew what the problem was. But I tried to fix it the wrong way, with literally only garbage comments. It also seems like, I didn't knew ctrl+f existed :D. Anyway, the clear solution for that problem is properly structuring the project. Each feature should get its own folder where views, business logic and routing should stay. With this, finding function should be easy.

Automated Tests

Obviously I didn't wrote tests. Why should I? Most of the YouTube tutorials only specified how to write something, not how to write good maintainable code. Tests verify that your code really works. Here is a good summary why writing tests rocks! stackoverflow: Is Unit Testing worth the effort?

In gotify/server I've written unit, integration and end to end tests from the start. It is such a nice feeling to add a feature and still know that the rest still works as intended, because the code is properly tested.

Security

SQL Injection

Basically every SQL statement in that project is vulnerable. Example, email verification:

mysql_query("UPDATE user SET activated = '1' WHERE id='".$_GET['code']."'");

$_GET['code'] comes from an query parameter.
The user can type anything in it. Like f.ex. malicious SQL like irrelevant' OR 1 = 1 --.
This would lead to the following query:

UPDATE user SET activated = '1' WHERE id='irrelevant' OR 1 = 1 -- '

After executing this query, every email would be verified, without actually receiving the email.

I've found one query where I used mysql_real_escape_string.

mysql_query("INSERT INTO post_comment ( post_id, name, comment, date) VALUES ('"
   .$post_id."','".$id."','".mysql_real_escape_string($message)."','"
   .$date."')")or die(mysql_error());

Escaping the message is good, but $id is also user supplied. I think I can safely say, that nearly all inputs on that website are vulnerable.

SQL Injection is on place 1 on the OWASP top ten security risks. Without knowledge it is pretty easy to allow SQL injection and with little knowledge it is also easy to prevent it.

The cure are prepared statements. These statements prevent the injection of sql.

In PHP prepared statements can be used via PDO. The above mysql_query can be fixed like this.

$pdo = new PDO('mysql:host=myhost;dbname=social', 'root', 'password');

$statement = $pdo->prepare("UPDATE user SET activated='1' WHERE id=?");
$statement->execute([$_GET['code']]);

Cross-Site Request Forgery (CSRF)

In a CSRF attack, the attacker tries to let the victim submit a malicious web request without knowing it. This could be to gain access to administration stuff or in my case add a new post to the timeline.

Here a vulnerable form:

<form action="index.php" method="post">
    <textarea name="posttext"></textarea>
    <input type="submit" name="posten" value="Posten">
</form>
<?php
if ($_POST["posten"]) {
    $text = $_POST["posttext"];
    if ($text != "") {
        post_schreiben($text, $userid);
    } else echo "<p id='falsered'>Content my not be empty</p>";
}
?>

This following website submits the malious form instantly after visiting the page, the user only have to navigate to this page, and then the message INJECT MESSAGE will be posted on the timeline.

<!DOCTYPE html>
<html>
<body>
    <form action="http://example.org/index.php" method="post">
        <input type="hidden" name="posttext" value="INJECT MESSAGE"/>
        <input type="hidden" name="posten" value="nah"/>
    </form>
    <script>
        document.forms[0].submit();
    </script>
</body>
</html>

Guarding against CSRF is somewhat more complicated. One way is to add an anti forgery token to the form. This token is generated on the server thus, can't be send from the malious website because it has no knowledge of it. After receiving a from post request from a client, the server then validates the anti forgery token and only then executes the request.

Pseudo code for a fix.

<?php
$token = createNewCSRFToken()
?>
<form action="index.php" method="post">
    <textarea name="posttext"></textarea>
    <input type="hidden" name="csrf_token" value="<?=$token?>"
    <input type="submit" name="posten" value="Posten">
</form>
<?php
if ($_POST["posten"]) {
    if (checkIfCSRFTokenIsValid($_POST['csrf_token'])) {
        $text = $_POST["posttext"];
        if ($text != "") {
            write_post($text, $userid);
        } else echo "<p id='falsered'>Content my not be empty</p>";
    } else echo "<p id='falsered'>Something went wrong try again.</p>";
}
?>

Cross-Site-Scripting (XSS)

Cross-Site-Scripting means the attacker injects HTML/JavaScript into the website. The vulnerable part of my project is the comment section of posts.
The content of the actual post is secured, guess I partly knew what I was doing.

if ($_GET["comment"] != "") {
    if (comment_post($userid, $_GET["comment"], $_GET["post_id"])) {
        echo "Success";
    } else echo "Something went wrong";
} else echo "Comment may not be empty";

To secure the script above, we need to sanitize $_GET["comment"]. In this case htmlentities can be used. This function converts all applicable characters to HTML entities.

F.ex. <script> will be converted to <script>. This prevents injecting html into the content of the comment.

if ($_GET["comment"] != "") {
    if (comment_post($userid, htmlentities($_GET["comment"]), $_GET["post_id"])) {
        echo "Success";
    } else echo "Something went wrong";
} else echo "Comment may not be empty";

User Passwords

Passwords aren't easy. This stackoverflow answer summarizes why passwords need to be secured how to secure them properly.

TL;DR: Use a cryptographic hash functions, with a salt and make it slow (:.

My project uses md5 as hash function without any salt. This is bad because it allows the attack with Rainbow Tables, this is a table with precomputed hashes, thus allowing a quick lookup of a hash.

Adding a salt would still not be enough, because md5 is to fast and can be easily brute forced.

PHP provides an simple api for creating and validating secure passwords hashes with good defaults.

$pw = "mypw"
$pwHash = password_hash($mypw, PASSWORD_DEFAULT);
// $pwHash = $2y$10$.vGA1O9wmRjrwAVXD98HNOgsNpDczlqm3Jq7KnEd1rVAGv3Fykk1a
// this hash contains the algorithm and other parameters
// the php default currently is BCrypt

if (password_verify($mypw, $pwHash)) {
    echo "SUCCESS";
}

Hardcoded Passwords

An application internally uses passwords, be it the database credentials or credentials for an online storage like aws.

Passwords like this should never be inserted plainly into the code because it is pretty easy to commit them into version control like git or make them browsable via a web server. See 1% of CMS-Powered Sites Expose Their Database Passwords.

Passwords should either be stored in environment variables or inside config files outside of the web root.

In this project, the credentials of the mysql database was stored plainly multiple times in different source files. There had also different passwords, tho there is a lot of unused code inside the project :D.

<?php
mysql_connect('mysql', 'root', 'password');
mysql_select_db('dbname');
?>

Serverside Authorization

Users should only be able to access/edit resources which they have permission for. F.ex. adding comments to a post from a user that isn't a friend should be prohibited.

Code without authorization check:

if ($_GET["comment"] != "") {
    if (comment_post($userid, $_GET["comment"], $_GET["post_id"])) {
        echo "Success";
    } else echo "Something went wrong";
} else echo "Comment may not be empty";

Code with authorization:

if ($_GET["comment"] != "") {
    if (postIsVisibleToUser($userid, $_GET["post_id"])) {
        if (comment_post($userid, $_GET["comment"], $_GET["post_id"])) {
            echo "Success";
        } else echo "Something went wrong";
    } else echo "Unauthorized";
} else echo "Comment may not be empty";

User Input

User input should never be trusted, and always be validated. This includes emails, dates, numbers and so on.

Not validating input can lead to bugs because some parts of the application may depend on valid inputs. A good example for this, is the registration. In there the user often adds an email address. If the users provides an invalid email address, intentionally or not the application should present the user an clear error message. Otherwise, password reset or similar functionality may not work.

WTF is this?

Register

function register($name, $npasswort, $email)
{
    $id1 = rand(1000000000, 9999999999);
    $id2 = rand(1000000000, 9999999999);
    $id3 = rand(1000000000, 9999999999);
    $id4 = rand(1000000000, 9999999999);
    db();
    $sql = mysql_query("SELECT * FROM user");
    $row = mysql_fetch_assoc($sql);
    if ($row['email'] != $email) {
        if ($row['id'] == $id1) {
            if ($row['id'] == $id2) {
                if ($row['id'] == $id3) {
                    if ($row['id'] == $id4) {

                    } else $id = $id4;
                } else $id = $id3;
            } else $id = $id2;
        } else $id = $id1;

        if ($id != "") {
            // insert user
        }
    }
}

Each user gets an unique id, this part tries to ensure that the id and email is unique. And well, it only checks against the first column inside the table. I say, at least one user will have a unique id. Having a duplicate email, is kinda bad, because it is used for logging in. If the ID is not unique, 4 random generated ids will be checked. If every id exists which is impossible, because only the first row will be checked, the script just does nothing.

Classes where they shouldn't be

class Login {
    protected $_email, $_password, $_result;

    public function __construct($email, $password) {
        $this->_email = $email;
        $this->_password = $password;
    }

    public function Login() {
        $db = new Database();
        if ($this->_email != "" or $this->_password != "") {
            if ($this->_email != "E-Mail" and $this->_password != "password") {
                if (login($this->_email, $this->_password)) {
                    // set user onto session
                    $this->_result = "Success";
                }
            } else $this->_result = "Fields may not be empty";
        } else $this->_result = "Fields may not be empty";
        $db->disconnect();
    }

    public function result() {
        return $this->_result;
    }
}

Usage:

$Login = new Login($form_email, $form_password);
$Login->Login();
echo $Login->result();

This is just a pretty bad example of using classes. Because it give no benefit but just bloats the code. The whole class should just be an function.

echo loginUser($form_email, $formPassword);

Conclusion

As expected my social network delivered, many security vulnerablities, bad code quality and some WTF moments. I enjoyed looking over it. It was a fun ride through the past :D. Thank you 13 year old me, for saving this project on cloud storage and even creating a sql dump from the database. Much appreciated.

Send Notifications to Android via REST-API

Jannis Mattheis — Sun, 11 Aug 2019 15:49:26 +0000

A while ago, I wanted to get notifications when certain events occur on my servers. This could be a SSH-Login or finishing a backup.

Back then, I started to self-host services to be less dependent on third-parties and to gain control over my data. After some research, I couldn't find any maintained open source project which had the functionality I wanted, so I started my own named Gotify.
Gotify is a pretty simple service written in Go that exposes a WebSocket endpoint which can be used to subscribe to newly posted messages. Gotify does not depend on any third-party service to function, thus does not use google services to deliver push notifications to your phone.

In this blog post, I'll show you how to set up Gotify and send some messages to it.

Setting Up Gotify

Gotify can be started via binary or Docker. In this tutorial, I'll use the provided Docker images as they are pretty easy to set up.

hub.docker.com/r/gotify/server

$ docker run -p 8080:80 \
             -v /var/gotify/data:/app/data \
             -e GOTIFY_DEFAULTUSER_PASS=secret \
             gotify/server:2.0.6

or as via docker-compose:

version: '3'

services:
  gotify:
    image: gotify/server:2.0.6
    ports:
      - 8080:80
    volumes:
      - "/var/gotify/data:/app/data"
    environment:
      GOTIFY_DEFAULTUSER_PASS: "secret"

(start with docker-compose up -d)

By default, Gotify uses SQLite as database. Thus, further simplifying the setup because no separate database is needed. SQLite should work well with a small user base, however if you have many concurrent users a different database may improve performance. Besides SQLite Gotify supports PostgreSQL and MySQL/MariaDB.

/app/data contains the database file (if SQLite is used), images for applications and other stuff. In this example the directory is mounted to /var/gotify/data this directory should be included in a backup.

-e GOTIFY_DEFAULTUSER_PASS=secret changes the password of the default user which will be created at startup.

Have a look at gotify.net/docs/config for all configuration options (like different database settings).

First Login / Definitions

By default, the default username/password is admin, however in this tutorial we changed the password to secret. With these credentials it's now possible to login into the WebUI at http://localhost:8080/ (use the port you specified while starting the docker container).

In the UI you can configure different things.

Clients: A client is a device or application that can manage other clients, messages and applications. However, a client is not allowed to send messages.

In this case your browser would be a client.

Applications: An application is a device or application that only can send messages.

An application could be a raspberry pi which notifies when it reboots.

Sending a message

You need an application to send messages to Gotify. Only the user who created the application is able to see its messages. An application can be added via:

WebUI: click the apps-tab in the upper right corner when logged in and add an application
REST-API: curl -u admin:secret -X POST https://yourdomain.com/application -F "name=test" -F "description=tutorial" See API-Docs

To authenticate as an application, you need the application token. The token is returned in the REST request and is viewable in the WebUI.

After copying the token you can simply use curl, HTTPie or any other http-client to push messages.

$ curl -X POST "http://localhost/message?token=<apptoken>" -F "title=my title" -F "message=my message" -F "priority=5"
$ http -f POST "http://localhost:8080/message?token=<apptoken>" title="my title" message="my message" priority="5"

Replace <apptoken> with your application token, it should look like this: AKTlZf.InA3uZHK.

priority currently only has an effect in the android app, 0 = not intrusive 10 = very intrusive.

The UI will render the message as plain text, it is possible to render it as markdown with extras.

You can use gotify/cli to push messages. The CLI stores url and token in a config file.

$ gotify push -t "my title" -p 10 "my message"
$ echo my message | gotify push

Install gotify/cli.

Android App

While the WebUI already creates notifications on new messages, Gotify also has an android app named gotify/android. It is available in the Play Store, on F-Droid and you can download the apk on the releases page. Setup is straight forward, enter the url to your Gotify instance and login.

Be aware: By default Android kills long-running apps as they drain the battery. With enabled battery optimization, Gotify will be killed, and you won't receive any notifications.

Here is one way to disable battery optimization for Gotify.

Open "Settings"
Search for "Battery Optimization"
Find "Gotify" and disable battery optimization

... and that's it! Thanks for reading my first blog post (:.