Security code review part 1

3 min readJul 3, 2021

Hello, I want to introduce you some simple examples of insecure code.
This article would be the first episode of the series ‘secure code review’.

The first vulnerable will be unsecured query concatenation in PHP.
Good coding practice says to ‘never use string concatenation with user control input value’.

SQL query concatenation

public void sqlQuery(){
  String query = "select id, firstname, lastname FROM authors";
   if (( firstname != NULL && firstname.length != 0 ) && ( lastname != NULL && lastname.length != 0 )) { 4 query += "WHERE forename = ? AND surname = ?";
  } else if ( firstname != NULL && firstname.length != 0 ) {
   query += "WHERE forename = ?";
  }else if ( lastname!= NULL && lastname.length != 0 ) {
   query += "WHERE surname = ?";
  }
  query += “;”;
  PreparedStatement pstmt = connection.prepareStatement( query )Information leak

The program logic will be fine when ‘firstname’ parameter or ‘lastname’ parameter is given. In case when neither were given, the SQL query would not have any WERE clause and the entire table would be returned.

Weak cryptography algorithm

In many of apps users sensetive data are stored in some types of databases, it’s very importat use uncompromise hashing algorithms when we are stored user data in databases. In this case developer use weak hash algorithm ‘SHA1’.
When attacker get access to the database, can break our hash function with vorious methods, and get users passwords.

public function userRegistration($user_login, $user_password){
  $mysql_query = "INSERT INTO app_users (user_login, user_password) values 
  " + ($user_login) + "" +  sha1($user_password)+";"
 }

Insecure Object References

The third case will by insecure direct object references. IDOR is a type of vulnerability that arises when an application uses input (controlled by user) to access objects directly

public void getData(){
  String query = "SELECT * FROM ids WHERE id = ?"; 
  PreparedStatement prepared = 
      connection.prepareStatement(query, ... );
  prepared.setString( 1, request.getParameter("id"));
  ResultSet results = prepared.executeQuery();
 }

In this case, we can simply modifies the “ID” parameter, and send whatever id number we want. Good security practice recommended verify and validate every user controlled input.

File extension bypassing

Uploaded files can be a significant risk to applications. In many applications, it would be the first testing case. After successfully upload, bypassing only needs to find a way to get the code executed.

public function fileUpload($user_login){
  if (!preg_match("/\.png/", $file)) {
   return "Extension do not allowed";
  } else {
   return "Success"
  }
 }

In this case, application verify the upload file extension. The function used a regular expression to check file type. Improper regular expression in many cases can be the weakness of file upload mechanism. In this method
programmer did not check the PNG extension is on the end of a file.
We can create a file with malicious content and extension .png.php.
To mitigate this case on the end of the regular expression, we have to add $ mark.

We can check our regular expression on various online tools
Impropper usages of regular expression

Regular expression with $ sign

Summary
It was the first part of a series about security code review, I hope you are enjoy it. Take care, and to next time.
References:
https://portswigger.net/web-security/access-control/idor
https://pentesterlab.com
https://owasp.org/www-project-secure-coding-practices-quick-reference-guide/migrated_content

Security code review part 1

Written by Dariusz Slusar