Insecure File Management

Why is this important?

Any functionality related to file management requires careful usage. If attackers are able to influence the input to file access related APIs, then it can have a serious impact. For example, attackers could read all the files on your application server.

In other cases, this can allow attackers to include their own code, or files, that are then executed by the application at runtime.

Check out this video for a high-level explanation on local file inclusion issues:

Fixing Insecure File Management

Option A: Migrate to Cloud Storage

1. Nowadays, there is rarely a need to allow users to interact with local files.
2. A better alternative is storing files in dedicated systems, such as AWS S3.
3. More information about the PHP AWS S3 SDK can be found here.
4. Other cloud providers offer similar options.

Option B: Properly Process User Input

1. Go through the issues that GuardRails identified in the PR.
2. A vulnerable example taken from DVWA is shown below:

<?php
if( isset( $_POST[ 'Upload' ] ) ) {
  // Where are we going to be writing to?
  $target_path  = DVWA_WEB_PAGE_TO_ROOT . "hackable/uploads/";
  $target_path .= basename( $_FILES[ 'uploaded' ][ 'name' ] );
  // Can we move the file to the upload folder?
  if( !move_uploaded_file( $_FILES[ 'uploaded' ][ 'tmp_name' ], $target_path ) ) {
    // No
    $html .= '<pre>Your image was not uploaded.</pre>';
  }
  else {
    // Yes!
    $html .= "<pre>{$target_path} succesfully uploaded!</pre>";
  }
}
?>

3. A secure version of this is also taken from DVWA:

<?php
if( isset( $_POST[ 'Upload' ] ) ) {
  // File information
  $uploaded_name = $_FILES[ 'uploaded' ][ 'name' ];
  $uploaded_ext  = substr( $uploaded_name, strrpos( $uploaded_name, '.' ) + 1);
  $uploaded_size = $_FILES[ 'uploaded' ][ 'size' ];
  $uploaded_type = $_FILES[ 'uploaded' ][ 'type' ];
  $uploaded_tmp  = $_FILES[ 'uploaded' ][ 'tmp_name' ];
  // Where are we going to be writing to?
  $target_path   = DVWA_WEB_PAGE_TO_ROOT . 'hackable/uploads/';
  $target_file   =  md5( uniqid() . $uploaded_name ) . '.' . $uploaded_ext;
  $temp_file     = ( ( ini_get( 'upload_tmp_dir' ) == '' ) ? ( sys_get_temp_dir() ) : ( ini_get( 'upload_tmp_dir' ) ) );
  $temp_file    .= DIRECTORY_SEPARATOR . md5( uniqid() . $uploaded_name ) . '.' . $uploaded_ext;
  // Is it an image?
  if( ( strtolower( $uploaded_ext ) == 'jpg' || strtolower( $uploaded_ext ) == 'jpeg' || strtolower( $uploaded_ext ) == 'png' ) &&
    ( $uploaded_size < 100000 ) &&
    ( $uploaded_type == 'image/jpeg' || $uploaded_type == 'image/png' ) &&
    getimagesize( $uploaded_tmp ) ) {
    // Strip any metadata, by re-encoding image (Note, using php-Imagick is recommended over php-GD)
    if( $uploaded_type == 'image/jpeg' ) {
      $img = imagecreatefromjpeg( $uploaded_tmp );
      imagejpeg( $img, $temp_file, 100);
    }
    else {
      $img = imagecreatefrompng( $uploaded_tmp );
      imagepng( $img, $temp_file, 9);
    }
    imagedestroy( $img );
    // Can we move the file to the web root from the temp folder?
    if( rename( $temp_file, ( getcwd() . DIRECTORY_SEPARATOR . $target_path . $target_file ) ) ) {
      // Yes!
      $html .= "<pre><a href='${target_path}${target_file}'>${target_file}</a> successfully uploaded!</pre>";
    }
    else {
      // No
      $html .= '<pre>Your image was not uploaded.</pre>';
    }
    // Delete any temp files
    if( file_exists( $temp_file ) )
      unlink( $temp_file );
  }
  else {
    // Invalid file
    $html .= '<pre>Your image was not uploaded. We can only accept JPEG or PNG images.</pre>';
  }
}
?>

4. Test it
5. Ship it 🚢 and relax 🌴

More information:

• Common Weakness Enumeration (CWE-73)