Skip to content

Commit

Permalink
Remove exec() from upload_file.inc
Browse files Browse the repository at this point in the history
  • Loading branch information
cpeel committed Oct 17, 2024
1 parent fad2262 commit ebafc10
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 16 deletions.
10 changes: 1 addition & 9 deletions SETUP/ci/check_security.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,6 @@

$basedir = $argv[1] ?? "../../";

// List of files that can contain system/exec calls
// TODO: Likely few or none of these should contain the calls either --
// opting for Symfony Process() instead -- but this ensures that no
// others are added from the current set until they get updated.
$ok_system_calls = [
"pinc/upload_file.inc",
];

// List of files that can contain mysqli_error() calls
$ok_mysqli_error_calls = [
"pinc/DPDatabase.inc",
Expand Down Expand Up @@ -48,7 +40,7 @@
}

// No file should include a system call (use Symfony Process instead)
if (file_includes_system_call("$basedir/$file") && !in_array($file, $ok_system_calls)) {
if (file_includes_system_call("$basedir/$file")) {
abort($file, "file includes system(), exec(), passthru(), shell_exec(), or escapeshellcmd()");
}

Expand Down
13 changes: 6 additions & 7 deletions pinc/upload_file.inc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
// <input type='file'> with name='uploaded_file' Then validate_uploaded_file()
// can be used to process the upload.

use Symfony\Component\Process\Process;

define("RESUMABLE_UPLOAD_SIZE", 1024 * 1024 * 1024); // 1GB

/**
Expand Down Expand Up @@ -262,13 +264,10 @@ function virus_check($file_path, $verbose)
// perform '$antivirus_executable -- <FILENAME>' and expect return
// value = 0. we use -- to not parse any further arguments starting
// with -/-- as options
$av_test_result = [];
$av_retval = 0;

$cmd = "$antivirus_executable -- " . escapeshellarg($file_path);
exec($cmd, $av_test_result, $av_retval);
// $av_retval == 0 is ok
if ($av_retval == 1) {
$process = new Process([$antivirus_executable, "--", $file_path]);
$av_retval = $process->run();
$av_test_result = explode("\n", $process->getOutput());
if (!$process->isSuccessful()) {
// Log the infected upload so that we can track user/frequency
$reporting_string = "upload_file.inc - Infected upload: " . $av_test_result[0];
error_log($reporting_string);
Expand Down

0 comments on commit ebafc10

Please sign in to comment.