The imperative role of code review

In order to ensure that the applications are developed and put into use in a way that prevents unauthorized access, data tampering, and theft, software developers utilize a set of techniques and best practices known as secure coding. Secure coding procedures must include security code reviews since they systematically examine software source code to find and correct flaws that compromise the security of the program.

By locating and correcting any flaws and vulnerabilities in the code, security code reviews can help developers create more secure applications and drastically lower the chance of security breaches. Additionally, as these reviews frequently result in the discovery and correction of bugs and other problems that are unrelated to security but may nonetheless result in software instability, subpar performance, and other difficulties, they can also enhance the general quality of the product.

Due to the sensitive nature of the data and resources that Android applications frequently manage and the large range of attack vectors that bad actors can use to breach Android devices and user data, code reviews are particularly important in the context of Android development.

Kick-off Meeting

The objectives, expectations, and scope of the code review should be clarified during a kick-off meeting between the developers and reviewers.

  • Communicate the objective of the code review
  • Agree on the timelines, milestones, and deliverables
  • Clarify the roles and responsibilities of each team member
  • Discuss any known areas of concern, potential risks, or challenges in the project

Code Review Checklist

To make sure that common problems, development standards, and best practices are effectively addressed throughout the review, create a code review checklist.

  • Coding standards and conventions: language-specific coding standards, proper use of comments and documentation, consistent naming conventions, and proper indentation
  • Architectural and design patterns: proper use of design patterns, separation of concerns, and adherence to the SOLID (Single responsibility, Open-closed, Liskov Substitution, Interface Segregation, and Dependency Inversion) principles
  • Code quality: efficient algorithms, optimized code, DRY (Don’t Repeat Yourself) principle, and proper error handling
  • Test coverage: unit tests, integration tests, and system tests
  • Security: adherence to secure coding practices, proper authentication and authorization mechanisms, secure storage of sensitive data, proper input validation and output encoding, and proper handling of exceptions and error messages

Security Code Review Process

A proper security code review process should include the following stages:

  1. Preparation: Information regarding the project being reviewed must be gathered before the review can start. This involves being aware of the technologies being used, the architecture of the application, the project documentation, and the security specifications. Having a detailed explanation of what must be developed and how it interacts with other modules is the minimum required. The reviewer should also define the review’s parameters and create an acceptable environment.
  • Recognize the goals and specifications of the project.
  • Familiarize yourself with the code architecture, modules, and components
  • Be familiar with the languages, libraries, and frameworks utilized in the project
  • Have a clear understanding of the security requirements of the project
  1. Static Analysis: During this step, the reviewer looks at the source code without actually running it to look for any potential security flaws, coding errors, or departures from accepted practices. This can be done manually or automatically with the aid of tools like static code analyzers (such as linters, Clang Static Analyzer, FindBugs for Java, etc.). Automated tools can save a lot of time by quickly locating problems in a big codebase, but manual code reviews are still required to find more subtle vulnerabilities that automated tools can miss. Static analysis would be performed on the code before requesting a code review in order to save time and effort and avoid any team efforts. Then, before the manual review, any errors, issues, etc. can be fixed.
  • Use static code analysis tools to identify common coding issues, such as unused variables, unreachable code, and overly complex functions
  • Use security analysis tools to identify potential vulnerabilities in the code, such as SQL injection, cross-site scripting, and insecure encryption
  1. Dynamic Analysis: In this stage, the reviewer concentrates on finding security flaws that can only be seen when the application is really being used, like runtime errors, inadequate input validation, or unsecured resource usage. This can be done manually using various testing approaches, such as penetration testing or fuzz testing, as well as dynamic analysis tools (such as debuggers, Valgrind, AddressSanitizer, etc.).
  • Use dynamic analysis tools to detect run-time issues such as memory leaks, buffer overflows, and thread safety issues
  • Use performance profiling tools to detect performance bottlenecks and resource-intensive operations in the code
  1. Post-Review: Following the static and dynamic analysis phases, the reviewer should list and rank the issues found, inform the development team about them, and help with their resolution. The reviewer should keep track of how the problems were resolved to make sure they had been fully fixed and hadn’t created any new vulnerabilities. This stage can lead to a request for change or approval of the code.
  1. Manual Code Review

The reviewers must perform a manual code review, going over all updates and modifications to the code while using the provided code review checklist. Reviewers should point out problems and offer suggestions at this phase to enhance the security of the code, its adherence to coding standards, and its general quality.

  • Go through the code changes and updates carefully
  • Verify the code’s compliance with the checklist
  • Take notes on any identified issues and discuss them with other reviewers, if necessary
  • Organize the feedback based on priority
  • Provide clear, respectful and constructive feedback
  • Provide examples or references if it is necessary
  • Ensure that the feedback is actionable
  1. Developer Updates and Follow-up

The developers should change the code in accordance with the input after identifying any faults and adding any proposed enhancements. The reviewers should do another round of reviews after the code upgrades to make sure all problems have been fixed.

  • Developers address the issues and update the code
  • Reviewers perform a follow-up review to ensure all issues have been resolved
  • Verify that the code review procedure has been completed.

Suppose we have a Java class for a simple calculator that performs basic arithmetic operations.

public class Calculator {
    public int add(int a, int b) {
        return a + b;
    }
    
    public int substract(int a, int b) {
        return a - b;
    }
    
    public int multiply(int a, int b) {
        return a * b;
    }
    
    public float divide(int a, int b) throws IllegalArgumentException {
        if (b == 0) {
            throw new IllegalArgumentException("Division by zero is not allowed");
        }
        return (float) a / (float) b;
    }
}

During the code review, the reviewer may identify the following issues:

  • Typo in the method names “substract” that should be “subtract.”
  • Lack of input validation, for instance, large input values or negative numbers
  • Lack of proper documentation comments for the methods

To address these issues, the developer updates the class as follows:

/**
 * Represents a simple calculator that performs basic arithmetic operations.
 */
public class Calculator {
    /**
     * Adds two integers and returns the result.
     *
     * @param a - first integer
     * @param b - second integer
     * @return the sum of a and b
     */
    public int add(int a, int b) {
        return a + b;
    }

    /**
     * Subtracts two integers and returns the result.
     *
     * @param a - first integer
     * @param b - second integer
     * @return the difference between a and b
     */
    public int subtract(int a, int b) {
        return a - b;
    }

    /**
     * Multiplies two integers and returns the result.
     *
     * @param a - first integer
     * @param b - second integer
     * @return the product of a and b
     */
    public int multiply(int a, int b) {
        return a * b;
    }

    /**
     * Divides two integers and returns the result.
     *
     * @param a - dividend integer
     * @param b - divisor integer
     * @return the division result of a and b
     * @throws IllegalArgumentException if divisor (b) is zero
     */
    public float divide(int a, int b) throws IllegalArgumentException {
        if (b == 0) {
            throw new IllegalArgumentException("Division by zero is not allowed");
        }
        return (float) a / (float) b;
    }
}
  1. Follow coding standards and best practices:

Following published coding standards and guidelines for the programming languages and frameworks being used is one of the greatest ways to assure code security. The likelihood of introducing security vulnerabilities can be significantly decreased by having a thorough awareness of the language nuances, their individual security dangers, and the accompanying best practices.

  1. Look for common security vulnerabilities:

It is important to be aware of vulnerabilities like injection flaws, issues with authentication and authorization, problems with data storage and transmission, problems with the use of cryptographic components, and memory management problems like buffer overflows while conducting the review.

  1. Understand the dependencies:

It is crucial to analyze both the team’s own code as well as any external dependencies (such as libraries or frameworks) that were applied to the project. Make sure the dependencies are current, secure, and used in accordance with best practices.

  1. Conduct threat modelling:

Reviewers need to be aware of the threat model for the application and comprehend any potential dangers and attack vectors. Making a threat map can assist the reviewer in focusing on the most important areas and setting priorities for the study.

  1. Use automated tools and perform manual reviews:

Automated tools are useful for identifying common vulnerabilities and concerns, but a manual review is necessary to find finer flaws that automated tools may miss.

Security Code Review Examples

Here are a few examples of common Android security issues in different programming languages:

...
public void saveToFileUnsafe(String fileName, String data)
        throws FileNotFoundException {
    FileOutputStream outputStream = openFileOutput(fileName, Context.MODE_PRIVATE);
    try {
        outputStream.write(data.getBytes());
    } catch (IOException e) {
        e.printStackTrace();
    } finally {
        try {
            outputStream.flush();
            outputStream.close();
        } catch (IOException e) {
-	         	e.printStackTrace();
        }
    }
}
...

In the above Java example, the method saveToFileUnsafe uses Context.MODE_PRIVATE to store sensitive data in an internal storage file. While using it makes the file private and inaccessible to other applications, it might still be accessible by an attacker if the device is rooted, and it doesn’t provide any encryption. A more secure approach would be to encrypt the data before storing it.

URL Injection

...
fun openUrl(url: String) {
    val webView = WebView(this)
    setContentView(webView)
    webView.loadUrl(url)
}
...

The function ‘openUrl’ in this Kotlin example accepts a URL string as input and opens it in a WebView. The input URL is not, however, validated or cleaned up. The WebView could potentially be used by a malicious party to run JavaScript code. Use a secure WebView setup and validate and sanitize the input URL to reduce the possibility of this happening. Additionally, disable JavaScript execution.

Security code reviews are a key component of creating secure software, especially for Android apps, which often handle private data and limited resources. To guarantee that the code they write is secure and resistant to common vulnerabilities and attack vectors, developers must adopt a thorough and systematic approach to conducting security code reviews, including both manual and automated analysis.

Developers can greatly increase the security of their code and create more durable and resilient apps by adhering to coding standards and best practices, learning about common security vulnerabilities, and successfully utilizing threat modelling and automation tools.

Link to Book: Secure Android Development: Best Practices for Robust Apps

Leave a Comment

Your email address will not be published. Required fields are marked *