这段代码能够防止SQL注入吗？

Question

背景

我被承包分析一个现有的数据提供者，我知道下面的代码存在问题；但是为了指出它有多糟糕，我需要证明它容易受到SQL注入攻击。

问题

哪个“Key”参数可以破坏PrepareString函数，并允许我执行一个DROP语句？

代码片段

Public Shared Function GetRecord(ByVal Key As String) As Record
    Dim Sql As New StringBuilder()

    With Sql
        .Append("SELECT * FROM TableName")
        If String.IsNullOrEmpty(Agency) Then
            .Append(" ORDER BY DateAdded")
        Else
            .Append(" WHERE Key = '")
            .Append(PrepareString(Key))
            .Append("'")
        End If
    End With

    Return ExecuteQuery(Sql.ToString())
End Function

Public Shared Function PrepareString(ByVal Value As String) As String
    Return Value.Replace("''", "'") _
                .Replace("'", "''") _
                .Replace("`", "''") _
                .Replace("´", "''") _
                .Replace("--", "")
End Function

- Josh Stodola

---

如果您正在使用VB，为什么要费力地想到每个可能的攻击向量，而不是使用参数化查询？ - Donnie

@Josh: 或许他们不想鼓励你避免使用最佳实践...如果你必须将PrepareString函数添加到“数十亿行代码”中，为什么不进行参数化呢？ - gbn

这段代码很糟糕。我想把它搞垮。我怎样才能搞垮它？我不是在询问如何从VB.NET正确查询数据库。我知道如何做到这一点，并且会使用参数化查询和/或存储过程。人们看到这个代码会惊慌失措，而不是阅读并回答实际问题。 - Josh Stodola

不想过于苛刻，但你的问题提得不太准确。你使用了严厉的措辞对待社区，结果引来了负面评价。我已经编辑了你的问题，以更符合问题本意，也许这样会赢得点赞，毕竟这是个好问题。 - George Stocker

@Shawn和Erik，我最初发表了一条有点粗鲁的评论（大概是“你们能不能回答这个该死的问题？”），因为人们在攻击我不使用参数化查询。这个问题曾经得分为-7，我认为完全是因为那个粗鲁的评论。重新叙述问题似乎解决了所有问题，现在我不认为还有人想关闭这个问题。 - Josh Stodola

Answer 1

回答您的直接问题：这段代码能防止SQL注入吗：不能。

以下是证明——将此字符串传递到PrepareString方法中：

Dim input = "'" & Chr(8) & "; Drop Table TableName; - " & Chr(8) & "-"
Dim output = PrepareString(input)

Console.WriteLine(input)
Console.WriteLine(output)

我修改了您发布的GetRecord方法，使其返回已准备好的完整SQL字符串，而不是从数据库中获取记录：

Console.WriteLine(GetRecord(output))

这是输出结果

Input  = ; Drop Table TableName; --
Output = '; Drop Table TableName; --
Query  = SELECT * FROM TableName WHERE Key = ''; Drop Table TableName; --'

增加1行额外的代码：

My.Computer.Clipboard.SetText(input)

你已经复制好了需要的字符串，可以直接粘贴到网站上的输入字段中以完成SQL注入：

'; Drop Table TableName; - -

[注意，由于 StackOverflow 在发布输出时省略了控制字符，因此您需要按照代码示例创建输出。]

运行 PrepareString 方法后，它将具有完全相同的输出 - Chr(8) ASCII 代码是退格符，它将删除您附加到我的额外 "'"，这将关闭您的字符串，然后我可以自由地添加任何想要的内容。您的 PrepareString 看不到我的 -- 因为我实际上使用 - - 带有一个退格字符来删除空格。

然后生成的 SQL 代码将执行我的 Drop Table 语句而不受阻碍，并立即忽略您查询的其余部分。

有趣的是您可以使用非可打印字符来基本上绕过您可以发明的任何字符检查。因此，最安全的方法是使用参数化查询（这不是您询问的内容，但这是避免此问题的最佳方法）。

- BenAlabaster

---

多少漏洞才能说服他？我可以整晚都这样做——我可以从你发送的原始页面数据生成一个POST并修改正确的表单值，以通过事件验证。然后我可以在我的Web浏览器中使用一个插件，使你的服务器将其视为同一会话。当然，替换Chr(8)可以修复此漏洞，但下一个漏洞呢？ - BenAlabaster

说实话，你要求的只是一些证明该方法存在缺陷的代码，我已经提供了这些代码，因此回答了你的问题。 - BenAlabaster

如果你的经理如此固执，任何数量的例子都无法说服他。他只会采取你上面使用的“好吧，也将chr(8)替换掉”的方式。你正在进行一场失败的战斗。 - Donnie

这个“证明”只是看起来利用了代码，因为控制台将chr(8)处理为实际的退格符。如果将这些字符放入字符串中并像您建议的那样复制到剪贴板中，它们仍然保持为字符串中的字符，并且不会吞噬任何字符。 - Jon Seigel

+1 给 @JonSeigel，这个答案是误导性的。据我所知，在 SQL Server 中，通过加倍引号来转义引号对于字符串参数是安全的，我没有看到任何一个例子破坏了这一点。另外需要注意的是，有一些合法的非参数化 SQL 用法，比如生成脚本，请在处理编码实践时不要采取“宗教”方法。 - Guillaume86

Answer 2

回答你的问题，不行。

.Replace("``", "''") 会阻止带有“`”的合法查询

.Replace("´", "''") 会阻止带有“´”的合法查询

.Replace("--", "") 会阻止带有“--”的合法查询

.Replace("''", "'") 会错误地修改带有“''''”的合法查询

此外，转义字符的完整集合因关系型数据库管理系统而异。参数化查询优胜。

- Yann Schwartz

---

同意，没有一种百分之百可靠的方法来避免查询字符串攻击。他们只会想出更好的方法。 - Neil N

传递的关键字值会导致我执行任何想要的SQL语句吗？ 我不担心无法工作的罕见查询，而是可能会造成严重损害的恶意查询。 - Josh Stodola

Josh，重点是人们发现了利用规范化问题的新方法，通过这种方式防止它们注定会失败。这就像试图机智地逃避HTML标记或文件路径一样。有时，在正确的位置插入一个空字符可以欺骗净化代码，等等。您必须依赖底层参数化机制，并祈祷它能够正常工作（或者在适当的时间进行修补）。 - Yann Schwartz

此外，正如我所说，清理函数确实会产生误报并破坏合法查询，这也是一个问题（现在尝试乘飞机就能感受到它有多么痛苦）。 - Yann Schwartz

+1 不要混淆无关的字符。只转义需要转义的字符。对于 SQL Server 和标准 ANSI SQL，这仅仅是字符串定界符 '。对于 MySQL 和偶尔出现的其他情况，反斜杠也可能成为问题。您还需要确保字符串中没有控制字符（MySQL 不喜欢 0 字节），但希望您已经从输入中过滤掉了这些字符。 - bobince

@Yann 我百分之百同意你的观点。不过，我希望你能给出一个例子，一个可以让我执行任何SQL语句的字符串。你看起来非常自信给定的代码可以实现这一点，所以请提供一个满足要求的“Key”参数样本，以充分说明其严重性。这是我唯一的要求。我知道这很糟糕，所以请帮我证明它有多糟糕。这样做是否太过分？ - Josh Stodola

Answer 3

我认为在SQL Server中是安全的，而且我也认为你实际上只需要做一个 s = s.Replace("'", "''")。当然，你应该使用参数化查询，但你已经知道这一点。

- erikkallen

Answer 4

我认为如果你只是将单引号替换为两个单引号，那么它就是无法被黑客攻击的。我听说有可能改变转义引号字符，并且这可能会破坏原来的保护机制，但我不确定。但我相信你是安全的。

- Shawn

Answer 5

这篇MSDN文章涵盖了大部分需要注意的内容（涉及到SQL注入时，恐怕不能说是所有）。

但是，我要重申其他人的建议：使用参数、参数和参数。

至于你的例子，有一些需要注意的事项[编辑：更新如下]：

• 字符串"1 OR 1=1"不会让用户获取所有信息

• 更糟糕的是"1; drop table sometablename"

根据该文章，您需要检查以下内容：

; - 查询定界符。

' - 字符数据字符串定界符。

-- - 注释定界符。

/* ... / - 注释定界符。服务器不评估/和*/之间的文本。

xp_ - 在扩展存储过程名称的开头使用，例如xp_cmdshell。

- brendan

---

如果是我在编写代码，你可以相信它就是这样的。 - Josh Stodola

在您的代码中更新了可能存在的“gotcha”，将尝试考虑更多。 - brendan

不，WHERE Key = '1 OR 1=1' 这个条件语句并不能让用户获取所有数据，因为在 '1 OR 1=1' 周围有引号。 - Jacob Krall

那篇文章中提供的建议非常不可取。如果攻击者能够从字符串文字中退出，那么删除分号是没有用的，因为在SQL语句中你还可以加入许多其他糟糕的内容。而如果它们无法从字符串文字中退出，因为引号被正确转义（或者希望如此，因为参数化正在生效），那么这只会损坏完全合法的输入。 - bobince

Bobince，看看文章中的第2个字符分隔符：' - 字符数据字符串分隔符。 - Stephen Curial

Answer 6

您正在尝试黑名单字符，以实现自己的SQL转义版本。我建议您查看this URL - SQL转义不一定是最糟糕的选择（例如快速修复现有应用程序），但需要正确执行以避免漏洞。

该URL链接到另一页，针对SQL Server的转义，作者提供了建议，可以帮助您避免漏洞而不限制功能。

如果有帮助的话，这些文章还建议转义括号（我称它们为方括号 - 但[]）。

- Mayo

---

不，我试图做的是破解这段代码。那就是我现在正在尝试做的事情。 - Josh Stodola

Answer 7

如果您使用您的代码，其他人可能会传递一个关键字(;select * from table;)，并获取他们想要的任何表格列表。

在您的代码中，您没有检查分号，这使您可以结束 t-sql 语句并开始另一个语句。

我建议使用参数化查询。

- Chuckie

---

我已经建立了一个测试环境来尝试破坏它。那个字符串没有做任何事情，只是因为无效的语法而抛出了异常。 - Josh Stodola