我的代码存在哪些安全问题？

Question

几年前，我在一个与PHP有关的问题上发布了答案，介绍了一种方法，让用户通过URI传递相对路径以下载文件，同时防止目录遍历。

但是，有些评论指出这段代码存在安全漏洞，并且有一些人给了负评（最近的一次是今天）。以下是该代码：

$path = $_GET['path'];
if (strpos($path, '../') !== false ||
    strpos($path, "..\\") !== false ||
    strpos($path, '/..') !== false ||
    strpos($path, '\..') !== false)
{
    // Strange things happening.
}
else
{
    // The request is probably safe.
    if (file_exists(dirname(__FILE__) . DIRECTORY_SEPARATOR . $path))
    {
        // Send the file.
    }
    else
    {
        // Handle the case where the file doesn't exist.
    }
}

我已经反复检查了代码并进行了测试，但仍然无法理解它引入的安全问题。

唯一在注释中得到的提示是../可以被%2e%2e%2f替换。这不是问题，因为PHP会自动将其转换为../。

这段代码存在什么问题？输入的值可能会允许目录遍历或以某种方式破坏某些东西。

- Arseni Mourzenko

---

“/etc/passwd” 仍然被接受。 - SLaks

@SLaks说：URI http://example.com?path=/etc/passwd 将会给出类似于 /home/demo-site/etc/passwd 的结果，因此不，这不是一个问题。 - Arseni Mourzenko

好问题。虽然我真的很想找到一个错误，但我还没有找到，而且那些得到所有赞的/etc/passwd建议真的很烦人...而且是错误的。安全性很好，但因为...嗯，因为不太酷而散布恐慌是不好的。我仍然对这段代码不满意（为什么没有类型检查？将其限制在某个白名单中等），但是出现这种膝反应的事实也很糟糕。 - Nanne

这个怎么会变成-2呢？这是一个严肃而清晰的问题，通过+5的评论可以看出，很多路人都会错，所以对于一些人来说并不是微不足道的。 - Nanne

你的代码应该从（使用原始问题中的路径）$path = '/whatever/path/' + $_GET['path']; 开始，然后你可以删除遍历并确保它仍在正确的文件夹中。（虽然你可能仍然会遇到 "%2e%2e%2f" 的问题） - Matthew Wilcoxson

@SLaks，乍一看似乎你是正确的，但实际上是错误的，因为/etc/password将被附加到/var/www/mysite或其他路径。你应该删除你的评论。 - SilverlightFox

Answer 1

还有许多其他可能性可能会被忽略，例如：

.htaccess
some-secret-file-with-a-password-in-it.php

换句话说，目录及其子目录中的任何内容都可以被访问，包括.htaccess文件和源代码。如果该目录或其子目录中的任何内容不应可下载，则存在安全漏洞。

- elixenide

---

这三条路径将导致位于基本目录内的文件。鉴于我引用的问题/答案的背景，我不认为这是一个安全问题。 - Arseni Mourzenko

我在问题中（或链接的问题和答案中）没有看到任何指示表明目录中的所有文件都可以下载。如果我漏掉了什么，请告诉我。然而，这是一个潜在的安全漏洞，因此这是问题的一个有效答案。 - elixenide

@Nanne 我已经修改了我的答案，实际上它基本上就是这个意思。 - elixenide

Answer 2

我想不出任何情况会导致这种方法失败。

然而，我不知道PHP的file_exists在内部是如何实现的，它是否有一些目前未知的怪异行为。就像PHP在5.3.4之前的某些文件系统函数中存在空字节相关问题一样。

因此，为了保险起见，我更愿意检查已解析的路径，而不是盲目信任PHP和我的假设，即提到的四个序列是唯一可能导致路径超出指定基本目录的序列。这就是为什么我更喜欢ircmaxell的解决方案而不是你的解决方案。

- Gumbo

Answer 3

我刚刚用Burp intruder测试了你的代码，但在这种情况下没有找到任何解决方案。

它可能因为利用其他/旧技术堆栈中类似的方法而被降级，通过黑名单列出某些字符组合。

正如您提到的，当前版本的PHP自动对输入进行URL解码，但存在一些缺陷，例如双重URL编码（点 = %252e），16位Unicode编码（点 = %u002e），超长UTF-8 Unicode编码（点 = %c0%2e）或插入空字节（%00）等技术，可以欺骗过滤器并允许服务器端代码将路径解释为未编码版本，一旦它得到了过滤器的认可。

这就是为什么它引起了警惕。即使您的方法在这里表现良好，通常情况下也可能不是这样。技术始终在变化，最好谨慎行事，尽可能使用对字符集解释免疫的技术，例如使用已知良好的字符白名单，或使用文件系统函数（链接答案中提到的realpath）来验证实际路径是否符合预期。

- SilverlightFox

Answer 4

黑名单是一个不好的习惯。你最好使用白名单（可以是允许的字面字符串或允许的字符）。

if(preg_match('/^[A-Za-z0-9\-\_]*$/', $path) ) {
    // Yay
} else {
    // No
}

或者，另一种选择是：

switch($path) {
    case 'page1':
    case 'page2':
        // ...
        break;
    default:
        $path = 'page1';
        break;
}

include $path;

- Scott Arciszewski