[码]这段upload的代码有漏洞没？

langyufeng

环境：PHP Version 4.4.0 AS4

1. function upload($file, $type = '', $path = '')
   
2. {
   
3.      global $_FILES, $Config;
   
4.    
   
5.      if ($_FILES[$file]['size'] > $Config['uploadMaxSize'])
   
6.     {
   
7.          return false;
   
8.          }
   
9.     $tmpExt = explode(".", $_FILES[$file]['name']);
   
10.      $tmpNum = count($tmpExt)-1;
    
11.      $t = strtolower($tmpExt[$tmpNum]);
    
12. 
    
13.     if(in_array($t, (explode('|', $type))) == true)
    
14.         {
    
15.          # copy file to server
    
16.         $filename = time() . "_" . rand(0, 10000) . '.' . $t;
    
17.          $file_path = $path . '/' . $filename;
    
18.         
    
19.         
    
20.          move_uploaded_file($_FILES[$file]['tmp_name'], $file_path);
    
21.         
    
22.          return $filename;
    
23.          }
    
24.     else
    
25.         {
    
26.          return false;
    
27.          }
    
28.    
    
29.     }

复制代码

1.      if ($_FILES['pic']['name'])
   
2.     {
   
3.          $pic = upload('pic', 'jpg|jpeg|gif|bmp|png', $Config['uppath'] . 'pic');
   
4.          }

复制代码

请大侠看看上面的代码有漏洞么？为什么？应该怎么改？
谢谢

放弃思考

有漏洞。你通过切割文件名来获得文件类型的方法不安全。
建议检测文件类型用检测MIME类型来判断，print_r($_FILE)一下你就明白了。
如果是图片类的文件。用getimagesize函数来判断很方便

langyufeng

ok
谢谢
改了一下，基本的应该没问题了，就是$_FILES[$file]['type'];依赖浏览器对文件类型的判断

1. 
   
2.          $mimetype = $_FILES[$file]['type'];
   
3.          $mimetypes = array(
   
4.     'image/bmp'        => 'bmp',
   
5.     'image/png'        => 'png',
   
6.         'image/gif'        => 'gif',
   
7.         'image/pjpeg'        => 'jpeg',
   
8.         'image/pjpeg'        => 'jpg'
   
9. );
   
10.          $t = $mimetypes[$mimetype];
    
11. 
    
12.     if(in_array($t, (explode('|', $type))) == true)
    

复制代码

gaoyikun

楼主and 2楼的，据本版有人说$_FILES['filename']['type']也是依赖文件扩展名来判断文件MIME类型的，所以我建议用mime_content_type()函数来判断：

1. 
   
2. $mimetype =mime_content_type($_FILES[$file]['tmp_name']);
   
3.          $mimetypes = array(
   
4.     'image/bmp'        => 'bmp',
   
5.     'image/png'        => 'png',
   
6.         'image/gif'        => 'gif',
   
7.         'image/pjpeg'        => 'jpeg',
   
8.         'image/pjpeg'        => 'jpg'
   
9. );
   
10.          $t = $mimetypes[$mimetype];
    
11. 
    
12.     if(in_array($t, (explode('|', $type))) == true)
    

复制代码

xuzuning

能否正确的判断出上传文件的类型并不是重要的问题
只要保存文件的目录没有执行权，至多只是显示的内容不正确

文件的后缀可以被修改，mime类型也只依赖与文件头，同样是可以被修改的

这个代码的真正问题在于
$filename = time() . "_" . rand(0, 10000) . '.' . $t;
1、rand只是产生随机数，并不能保证是唯一的。在同一时间有大量用户上传时就可能造成文件名相同
2、虽然使用time函数可以减少重名的可能性，但你并不能保证服务器时间不被被修改
3、move_uploaded_file和copy函数都不能改写已存在的文件，遇到同名文件将报错。于是就可能暴露你的服务器目录结构

langyufeng

mime_content_type函数服务器不支持，汗


设定存文件的目录没有执行权，想想重要的还是这个，嘿嘿，不能执行传了你也没用

1. 
   
2. <Directory "/www/htdocs/upload/">
   
3. php_flag engine off
   
4. </Directory>
   

复制代码

1. 
   
2. $filename = time() . "_" . rand(0, 10000) . '.' . $t;
   

复制代码

文件名问题应该不大吧，呵呵，我再对同名文件做一个判断吧

谢谢大家：）

gaoyikun

原帖由 xuzuning 于 2006-1-20 16:28 发表
这个代码的真正问题在于
$filename = time() . "_" . rand(0, 10000) . '.' . $t;
1、rand只是产生随机数，并不能保证是唯一的。在同一时间有大量用户上传时就可能造成文件名相同
2、虽然使用time函数可以减少重名的可能性，但你并不能保证服务器时间不被被修改
3、move_uploaded_file和copy函数都不能改写已存在的文件，遇到同名文件将报错。于是就可能暴露你的服务器目录结构

filename前再加上一个上传者的session_id，这样应该可以把重名的可能性降到最低了。

bkkkd

mime类型安全吗？
mime类型只是在文件的前头写上类型的名称，
谁不会写呀？

bkkkd

个人认为只要后缀不会变成.php就是安全了

spiceboy

通过mime判断用处也不是很大。很多都会判断错。
例如只能判断文本，而无法区分txt还是php

个人觉得：
用户文件扩展名是什么，你就给他保存为什么就够了。但是，如果用户上传.php.php3.php4.phtml.cgi.pl等你的apache当作scripts来解析的文件。就需要将其文件名称改一下。
例如.php改为._php。