Getting my Laravel application security audited

Vigilant is an open source web monitoring application written in PHP using the Laravel framework. It uses the latest PHP and Laravel version and minimizes dependencies. It has been a core design decision to minimize the amount of third party packages for Vigilant to prevent dependency hell. "This choice has also improved security—less external code means fewer potential abandoned dependencies and more control at the cost of some extra work. When I do add an external dependency I have a few requirements such as code quality, activeness and downloads.

Let's get into the security audit, each blockquote is a copy/paste from the report and not something I've typed myself.

Summary and Methodology

The report starts with an executive summary which comes with an issue chart and conclusion.

<Company> performed a source code review on the source code repository vigilant. The objective of the review was to identify code quality and security issues in the source code. A total of eight issues were identified during the review. Each issue was rated according to its severity: one was rated as low severity, and seven as informational.

Conclusions
The code review found one low-severity security issue in the codebase. This issue should be addressed to ensure the security of the application.

The overall quality of the codebase is good, but we have identified a few opportunities to improve the quality and maintainability of the code.

Overall, the codebase is in good shape.

The next page contains recommendations in which they say that there is no guarantee that there aren't any security vulnerabilities.

Recommendations

We recommend addressing the issues identified in this report as part of regular development activities. The severity of the issues should be taken into account when prioritizing the fixes.

While the review identified some issues, it is important to note that this assessment does not guarantee the absence of other security vulnerabilities in the application. It is recommended to perform regular security assessments to ensure the application remains secure.

After these two introductory pages the report starts by describing the application and how the audit was performed.

The vigilant application is written in PHP (13.2 KLOC) and JavaScript (97 LOC) and uses the Laravel framework.

Scope

The source code review was performed against the code found in https://github.com/govigilant/vigilant.

We focused on the parts of the application written in PHP. Source code written in other languages was not reviewed.

Methodology

The source code review was performed by analyzing the vigilant source code repository using static code analysis tools. The findings were then manually reviewed to ensure their accuracy and relevance. The code review focused on the following areas:

• Security vulnerabilities

• Code quality and maintainability

• Dependency management

• Documentation

One of the most interesting parts I found was a list of tools they used, this means that I could integrate these tools in a pipeline to automate some of these checks.

The following tools were used to perform the source code review:

• Composer (https://getcomposer.org)

• ComposerRequireChecker (https://github.com/maglnet/ComposerRequireChecker)

• Git (https://git-scm.com)

• PHPCPD (https://github.com/sebastianbergmann/phpcpd)

• PHPMD (https://phpmd.org)

• PHP_CodeSniffer (https://github.com/PHPCSStandards/PHP_CodeSniffer)

• Psalm (https://psalm.dev)

• Trivy (https://trivy.dev)

What are the findings?

There are a total of eight issues identified, one of them was low severity and seven were informational.

Docker image runs as root

The only low severity finding was that the docker image runs as root. I found it really nice that they provide the impact, recommendation and references to understand the issue.

Missing strict_types declaration

The second finding is that all the PHP files are missing the strict_types declaration. This is not really an issue since I'm running PHPStan in Github actions to check for type errors.

Commented-out code

Here you instantly notice that the tool isn't made specifically for Laravel, it complains about comments in configuration files. It did find two places where commented code existed but then other 24 cases were configuration files.

Unresolved TODO comments

Here is a valid point in my opinion, the codebase should not contain any TODO comments. They are only acceptable if there is an action planned to resolve them, such as a story on the backlog.

Error suppression with the @ operator

Vigilant has a built-in crawler which reads HTML, sometimes HTML can be malformed which is why I've chosen to use the @ operator. This was a conscious decision and I've included a comment why it was added.

High cyclomatic complexity

It's nice that the tool checks this but you do not always have control over how it must be implemented. For example, their first finding in the table needs to be implemented a certain way because that package requires it like that.

Short commit messages

Vigilant is currently only developed by me which made me not really pay attention to the commit messages. I've since changed this by adding new code via pull requests with a clear description.

No healthcheck defined in Dockerfile

I've added health checks in the docker compose, but not in the Dockerfile as the image is intended to only run via compose. I'm using the same image for the web container and the queue container so I'd need multi-stage builds to add separate health checks which are not my priority right now. It would be a good addition.

My feedback on the tool

As this audit was done for free I was asked to provide feedback on the findings which I gladly did.

First of all I agree with the strict types finding but I would like to have seen a note that the files are being checked by PHPStan. Also, the commented out code for Laravel projects in the config directory findings can be omitted.

In the list of tools was Trivy, it would have been nice to have somewhere mentioned that the CVE's have been scanned and none matched.

Finally I've suggested that some checks on composer dependencies should be added, for example if there are low quality or abandoned external packages in the project I would like to know. The tool should also run its checks on these dependencies to get an idea of the quality.
A package that has a few hundred downloads is much less reliable than a package that has thousands of downloads on Packagist as the chance of a high quality fork is higher when the original author decides to stop development.

Final Thoughts

I'm happy that there aren't serious issues in Vigilant's source code, it was interesting to see what things are checked and how. This review confirmed the value of key design decisions like limiting dependencies.

If you found this interesting, please consider starring the repository 🙏