Solution for the vulnerable code snippet n°7

par | Sep 4, 2023 | Articles techniques, Challenges

The vulnerability to be detected for this challenge was a local file disclosure due to incorrect path limitation. Solving the challenge doesn’t require any special knowledge, just reading the Actix web framework documentation to understand how the methods behave.

Note: This article is also available in french 🇫🇷. The challenge was announced in this tweet 🐦.

Explanation

An application can serve dynamic pages, but also static files (CSS, JavaScript, images, etc.).
Here, the application displays the image /public/static/polygons.svg on the home page. This image is served using the r#static() function under the /public/ route. However, the r#static() function doesn’t perform any filtering and simply returns the file passed as an argument. At route level, filtering could be set using a regular expression to match certain file types only. However, this is not the case: the regular expression .* is used, allowing any file type. There are no restrictions on file type, and an attacker can request any file on the system that is readable with the application’s permissions.

It is, for example, possible to go up the tree to read files outside the web server root.

➜ curl 'http://127.0.0.1:8888/public/../../../../../../etc/os-release' --path-as-is
NAME="Arch Linux"
PRETTY_NAME="Arch Linux"
ID=arch
BUILD_ID=rolling
ANSI_COLOR="38;2;23;147;209"
HOME_URL="https://archlinux.org/"
DOCUMENTATION_URL="https://wiki.archlinux.org/"
SUPPORT_URL="https://bbs.archlinux.org/"
BUG_REPORT_URL="https://bugs.archlinux.org/"
PRIVACY_POLICY_URL="https://terms.archlinux.org/docs/privacy-policy/"
LOGO=archlinux-logo

Or read the application’s source code.

➜ curl 'http://127.0.0.1:8888/public/examples/app-vuln.rs'
use actix_files::NamedFile;
use actix_web::{get, HttpRequest, HttpResponse, Responder, Result};
use std::path::PathBuf;

#[get("/")]
async fn index() -> impl Responder {
  let html = "<!doctype html><html><body><h1>Polygons!</h1><img src=\"/public/static/polygons.svg\"></body></html>";
  HttpResponse::Ok().body(html)
}

async fn r#static(req: HttpRequest) -> Result<NamedFile> {
  let path: PathBuf = req.match_info().query("filename").parse().unwrap();
  Ok(NamedFile::open(path)?)
}

#[actix_web::main]
async fn main() -> std::io::Result<()> {
  use actix_web::{web, App, HttpServer};

  HttpServer::new(||
    App::new()
      .service(index)
      .route("/public/{filename:.*}", web::get().to(r#static))
    )
    .bind(("127.0.0.1", 8888))?
    .run()
    .await
}

The Actix documentation, warns of the danger of this kind of use case:

Matching a path tail with the [.*] regex and using it to return a NamedFile has serious security implications. It offers the possibility for an attacker to insert ../ into the URL and access every file on the host that the user running the server has access to.

However, using a more restrictive regular expression is not enough. If we change /public/{filename:.*} to /public/{filename:static/.+\.svg} this improves things considerably by preventing all file types from being queried.

  • ✅ curl 'http://127.0.0.1:8888/public/static/polygons.svg' (begins with static/ and ends with .svg)
  • ❌ curl 'http://127.0.0.1:8888/public/examples/app-vuln.rs' (begins with examples/ and ends with .rs)

At first glance, you might think that’s enough, but it’s always possible to go back up the file tree to read other files of the same type outside the web server root.

  • ✅ curl 'http://127.0.0.1:8888/public/static/../../../../../../home/noraj/Pictures/logo_acceis_black.svg' --path-as-is (begins with static/ and ends with .svg)

In a real application, the regular expression might have whitelisted common static file extensions: .js, .css, .xml, .png, …

The .js could be used to read a config.js file from another application, the .png could be used to read users’ private images, since the route may or may not allow certain file names, but it’s no substitute for access control based on user permissions, the .xml, which would legitimately allow access to the sitemap.xml, could also be used to retrieve sensitive configuration files, and so on.

Fixed code

Here is the corrected code:

Fixed code

Using a regular expression as a routing rule is definitely not the right way to go about implementing access control.

To serve static files, it’s simpler and more secure to serve a specific folder. App::service() will ensure that it’s not possible to go outside this folder.

However, this involves serving public files without access control (stylesheets, scripts, etc.). The mistake not to be made would be to position an upload/ directory, allowing users to upload their personal files, as a static route since everyone would have access to it. Unfortunately, this is often the case when the developer wants to give access to the company’s public files (e.g. whitepaper.pdf, annual-results.pdf) which are located in upload/, not realizing that by giving access to this folder, he is also giving access to upload/users/toto/document-discussion-private.odt. You must therefore ensure that all folders served by static file routes are public, and manage confidential files with an access control mechanism, even if they are static.

Diff

The source code is available on the Github repository Acceis/vulnerable-code-snippets and on the website acceis.github.io/avcs-website.

About the author

Article written by Alexandre ZANNI aka noraj, Penetration Testing Engineer at ACCEIS.