Commit 7d8daad4271eae637322a789a5733d602ee5f8ae

Authored by Megan Watson
1 parent d4782b2f

Security fix for webservice uploads using a relative path.

Committed by: Megan Watson
ktwebservice/KTUploadManager.inc.php
@@ -105,15 +105,27 @@ class KTUploadManager @@ -105,15 +105,27 @@ class KTUploadManager
105 105
106 $check = ($tempdir == $main_temp_dir); 106 $check = ($tempdir == $main_temp_dir);
107 107
  108 + /*
  109 + Removing the return, if the file is not directly in the temp directory then it may be a security risk, for instance a file can be uploaded using the following tempfilename: /var/www/var/uploads/../../../../etc/passwd
  110 + Checking the basename of the file should negate this risk.
108 if($check){ 111 if($check){
109 return $check; 112 return $check;
110 } 113 }
  114 + */
111 115
112 // in case of a symlinked directory, check if the file exists and is in the uploads directory 116 // in case of a symlinked directory, check if the file exists and is in the uploads directory
113 $file = basename($tempfilename); 117 $file = basename($tempfilename);
114 $path = $this->temp_dir . DIRECTORY_SEPARATOR . $file; 118 $path = $this->temp_dir . DIRECTORY_SEPARATOR . $file;
115 119
116 if(file_exists($path)){ 120 if(file_exists($path)){
  121 +
  122 + // Added check - if file name contains ../ to get down a few levels into the root filesystem
  123 + if(strpos($tempfilename, '../') !== false){
  124 + global $default;
  125 + $default->log->error('Upload Manager: temporary filename contains relative path: '.$tempfilename .' could be attempting to access root level files');
  126 + return false;
  127 + }
  128 +
117 return true; 129 return true;
118 } 130 }
119 131
@@ -122,12 +134,6 @@ class KTUploadManager @@ -122,12 +134,6 @@ class KTUploadManager
122 $default->log->error('Upload Manager: can\'t resolve temporary filename: '.$tempfilename .' in uploads directory: '.$this->temp_dir); 134 $default->log->error('Upload Manager: can\'t resolve temporary filename: '.$tempfilename .' in uploads directory: '.$this->temp_dir);
123 135
124 return false; 136 return false;
125 -  
126 - /*  
127 - $tempdir = substr($tempfilename,0,strlen($this->temp_dir));  
128 - $tempdir = str_replace('\\','/', $tempdir);  
129 - return ($tempdir == $this->temp_dir);  
130 - */  
131 } 137 }
132 138
133 function store_base64_file($base64, $prefix= 'sa_') 139 function store_base64_file($base64, $prefix= 'sa_')