developer tip

매개 변수가 문자열이 아닌 경우 SQL 조회를 매개 변수화하지 않는 것이 안전합니까?

optionbox 2020. 7. 24. 20:38
반응형

매개 변수가 문자열이 아닌 경우 SQL 조회를 매개 변수화하지 않는 것이 안전합니까?


SQL 인젝션 측면 에서 매개 변수를 매개 변수화해야 할 필요성을 완전히 이해합니다 string. 그것은이 책에서 가장 오래된 트릭 중 하나입니다. 그러나 언제 매개 변수화 하지 않는 것이 정당화 될 수 SqlCommand있습니까? 매개 변수화하지 않는 "안전한"데이터 유형이 있습니까?

예를 들어 : 나는 SQL 전문가 근처 에 있다고 생각하지 않지만 SQL 주입에 잠재적으로 취약하여 a bool또는 a를 받아 들여 int쿼리에 바로 연결할 수 있는 경우는 생각할 수 없습니다 .

내 가정이 맞습니까, 아니면 내 프로그램에 큰 보안 취약점을 남길 수 있습니까?

명확하게하기 위해이 질문은 으로 태그 가 지정됩니다. 내가 말할 때 "매개 변수를"같은 생각 public int Query(int id) .


기술적으로 안전하다고 생각 하지만 들어가는 것은 끔찍한 습관입니다. 정말로 이런 쿼리를 작성하고 싶습니까?

var sqlCommand = new SqlCommand("SELECT * FROM People WHERE IsAlive = " + isAlive + 
" AND FirstName = @firstName");

sqlCommand.Parameters.AddWithValue("firstName", "Rob");

또한 유형이 정수에서 문자열로 변경되는 상황에 취약하게 만듭니다 (이름에도 불구하고 문자를 포함 할 수있는 직원 번호를 생각하십시오).

따라서 EmployeeNumber 유형을에서 (으) int변경 string했지만 SQL 쿼리를 업데이트하지 않았습니다. 죄송합니다.


웹 서버와 같이 제어하는 ​​컴퓨터에서 강력한 형식의 플랫폼을 사용하는 bool경우 DateTime, 또는 int(및 기타 숫자) 값만있는 쿼리에 대한 코드 삽입을 방지 할 수 있습니다 . 중요한 것은 SQL Server가 모든 쿼리를 다시 컴파일하도록 강요하고 어떤 빈도로 실행되는 쿼리에 대한 좋은 통계를 얻지 못하게함으로써 (캐시 관리를 손상시키는) 성능 문제입니다.

그러나 "제어하는 컴퓨터에서"부분은 중요합니다. 그렇지 않으면 사용자가 임의의 텍스트를 포함하도록 해당 값에서 문자열을 생성하기 위해 시스템에서 사용하는 동작을 변경할 수 있기 때문입니다.

또한 장기적으로 생각하고 싶습니다. 오늘날의 구식이자 강력한 형식의 코드 기반이 자동 번역을 통해 새로운 인기 동적 언어로 이식되어 갑자기 유형 검사를 잃어 버렸지 만 동적 코드에 대한 올바른 단위 테스트가 아직 완료되지 않은 경우 어떻게됩니까 ?

실제로 이러한 값에 쿼리 매개 변수를 사용하지 않을 이유는 없습니다. 이 문제를 해결하는 올바른 방법 입니다. 실제로 상수 일 때 SQL 문자열에 값을 하드 코딩하십시오. 그렇지 않으면 매개 변수를 사용하는 이유는 무엇입니까? 힘들지 않아요

궁극적으로, 나는 이것을 버그 라고 부르지 않지만, 나는 그것을 냄새 라고 부를 것입니다 : 그 자체로는 버그가 거의 떨어지지 않지만 버그가 근처에 있거나 결국에있을 것이라는 강한 표시입니다. 좋은 코드는 냄새를 남기지 않으며 좋은 정적 분석 도구는 이것을 표시합니다.

불행히도 이것이 바로 이길 수있는 논쟁이 아니라고 덧붙입니다. "올바른"상태가 더 이상 충분하지 않은 상황처럼 들리며,이 문제를 스스로 해결하기 위해 동료의 발끝을 밟아도 팀의 역 동성이 향상되지는 않습니다. 결국 도움보다 더 많은 상처를 줄 수 있습니다. 이 경우 더 나은 방법은 정적 분석 도구 사용을 촉진하는 것입니다. 그것은 기존의 코드를 목표로하고 돌아가고 수정하는 노력에 정당성과 신뢰성을 부여 할 것입니다.


경우에 따라 문자열 값 이외의 매개 변수화되지 않은 (연결된) 변수를 사용하여 SQL 주입 공격을 수행 할 수 있습니다. Jon의이 기사 참조 : http://codeblog.jonskeet.uk/2014/08/08/the-bobbytables 문화 / .

일 때이다 ToString라고, 일부 사용자 지정 문화 공급자가 쿼리에 어떤 SQL을 주입의 문자열 표현으로 문자열이 아닌 매개 변수를 변환 할 수 있습니다.


입니다 하지 심지어 문자열이 아닌 유형에 대한 안전합니다. 항상 매개 변수를 사용하십시오. 기간.

다음 코드 예제를 고려하십시오.

var utcNow = DateTime.UtcNow;
var sqlCommand = new SqlCommand("SELECT * FROM People WHERE created_on <= '" + utcNow + "'");

첫눈에 코드는 안전 해 보이지만 Windows 국가 별 설정에서 일부 내용을 변경하고 간단한 날짜 형식으로 주입을 추가하면 모든 것이 변경됩니다.

날짜 시간 주입

이제 결과 명령 텍스트는 다음과 같습니다.

SELECT * FROM People WHERE created_on <= '26.09.2015' OR '1'<>' 21:21:43'

int사용자가 SQL 주입으로 쉽게 변경할 수있는 사용자 정의 음수 부호를 정의 할 수 있으므로 유형에 대해서도 동일하게 수행 할 수 있습니다.

현재 문화 대신 불변 문화를 사용해야한다고 주장 할 수는 있지만 이와 같은 문자열 연결을 여러 번 보았으므로 문자열을 사용하여 문자열을 연결할 때 놓치기 쉽습니다 +.


"SELECT * FROM Table1 WHERE Id ="+ intVariable.ToString ()


보안
괜찮습니다.
공격자는 입력 한 int 변수에 아무것도 주입 할 수 없습니다.

성능이
좋지 않습니다.

매개 변수를 사용하는 것이 좋습니다. 따라서 쿼리는 한 번 컴파일되고 다음 사용을 위해 캐시됩니다. 다음에 다른 매개 변수 값을 사용하더라도 쿼리가 캐시되므로 데이터베이스 서버에서 컴파일 할 필요가 없습니다.

Coding Style
Bad practice.

  • Parameters are more readable
  • Maybe it makes you get used to queries without parameters, then maybe you made a mistake once and use a string value this way and then you probably should say goodbye to your data. Bad habit!


"SELECT * FROM Product WHERE Id=" + TextBox1.Text


Although it is not your question, but maybe useful for future readers:

Security
Disaster!
Even when the Id field is integer, your query may be subject to SQL Injection. Suppose you have a query in your application "SELECT * FROM Table1 WHERE Id=" + TextBox1.Text, An attacker can insert into text box 1; DELETE Table1 and the query will be:

"SELECT * FROM Table1 WHERE Id=1; DELETE Table1"

If you don't want to use parametrized query here, you should use typed values:

string.Format("SELECT * FROM Table1 WHERE Id={0}", int.Parse(TextBox1.Text))


Your Question


My question arose because a coworker wrote a bunch of queries concatenating integer values, and I was wondering whether it was a waste of my time to go through and fix all of them.

I think changing those codes is not waste of time. Indeed change is Recommended!

if your coworker uses int variables, it has no security risk , But I think changing those codes is not waste of time and indeed changing those codes is recommended. It makes code more readable, more maintainable and makes execution faster.


There are actually two questions in one. And question from the title has very little to do with concerns expressed by the OP in the comments afterwards.

Although I realize that for the OP it is their particular case that matters, for the readers coming from Google, it is important to answer to the more general question, that can be phrased as "is concatenation as safe as prepared statements if I made sure that every literal I am concatenating is safe?". So, I would like to concentrate on this latter one. And the answer is

Definitely NO.

The explanation is not that direct as most readers would like, but I'll try my best.

I have been pondering on the matter for a while, resulting in the article (though based on the PHP environment) where I tried to sum everything up. It occurred to me that the question of protection from SQL injection is often eludes toward some related but narrower topics, like string escaping, type casting and such. Although some of the measures can be considered safe when taken by themselves, there is no system, nor a simple rule to follow. Which makes it very slippery ground, putting too much on the developer's attention and experience.

The question of SQL injection cannot be simplified to a matter of some particular syntax issue. It is wider than average developer used to think. It's a methodological question as well. It is not only "Which particular formatting we have to apply", but "How it have to be done" as well.

(From this point of view, an article from Jon Skeet cited in the other answer is doing rather bad than good, as it is again nitpicking on some edge case, concentrating on a particular syntax issue and failing to address the problem at whole.)

When you're trying to address the question of protection not as whole but as a set of different syntax issues, you're facing multitude of problems.

  • the list of possible formatting choices is really huge. Means one can easily overlook some. Or confuse them (by using string escaping for identifier for example).
  • Concatenation means that all protection measures have to be done by the programmer, not program. This issue alone leads to several consequences:
    • such a formatting is manual. Manual means extremely error prone. One could simply forget to apply.
    • moreover, there is a temptation to move formatting procedures into some centralized function, messing things even more, and spoiling data that is not going to database.
  • when more than one developers involved, problems multiply by a factor of ten.
  • when concatenation is used, one cannot tell a potentially dangerous query at glance: they all potentially dangerous!

Unlike that mess, prepared statements are indeed The Holy Grail:

  • it can be expressed in the form of one simple rule that is easy to follow.
  • it is essentially undetacheable measure, means the developer cannot interfere, and, willingly or unwillingly, spoil the process.
  • protection from injection is really only a side effect of the prepared statements, which real purpose is to produce syntactically correct statement. And a syntactically correct statement is 100% injection proof. Yet we need our syntax to be correct despite of any injection possibility.
  • if used all the way around, it protects the application regardless of the developer's experience. Say, there is a thing called second order injection. And a very strong delusion that reads "in order to protect, Escape All User Supplied Input". Combined together, they lead to injection, if a developer takes the liberty to decide, what needs to be protected and what not.

(Thinking further, I discovered that current set of placeholders is not enough for the real life needs and have to be extended, both for the complex data structures, like arrays, and even SQL keywords or identifiers, which have to be sometimes added to the query dynamically too, but a developer is left unarmed for such a case, and forced to fall back to string concatenation but that's a matter of another question).

Interestingly, this question's controversy is provoked by the very controversial nature of Stack Overflow. The site's idea is to make use of particular questions from users who ask directly to achieve the goal of having a database of general purpose answers suitable for users who come from search. The idea is not bad per se, but it fails in a situation like this: when a user asks a very narrow question, particularly to get an argument in a dispute with a colleague (or to decide if it worth to refactor the code). While most of experienced participants are trying to write an answer, keeping in mind the mission of Stack Overflow at whole, making their answer good for as many readers as possible, not the OP only.


Let's not just think about security or type-safe considerations.

The reason you use parametrized queries is to improve performance at the database level. From a database perspective, a parametrized query is one query in the SQL buffer (to use Oracle's terminology although I imagine all databases have a similar concept internally). So, the database can hold a certain amount of queries in memory, prepared and ready to execute. These queries do not need to be parsed and will be quicker. Frequently run queries will usually be in the buffer and will not need parsing every time they are used.

UNLESS

Somebody doesn't use parametrized queries. In this case, the buffer gets continually flushed through by a stream of nearly identical queries each of which needs to be parsed and run by the database engine and performance suffers all-round as even frequently run queries end up being re-parsed many times a day. I have tuned databases for a living and this has been one of the biggest sources of low-hanging fruit.

NOW

To answer your question, IF your query has a small number of distinct numeric values, you will probably not be causing issues and may in fact improve performance infinitesimally. IF however there are potentially hundreds of values and the query gets called a lot, you are going to affect the performance of your system so don't do it.

Yes you can increase the SQL buffer but it's always ultimately at the expense of other more critical uses for memory like caching Indexes or Data. Moral, use parametrized queries pretty religiously so you can optimize your database and use more server memory for the stuff that matters...


To add some info to Maciek answer:

It is easy to alter the culture info of a .NET third party app by calling the main-function of the assembly by reflection:

using System;
using System.Globalization;
using System.Reflection;
using System.Threading;

namespace ConsoleApplication2
{
  class Program
  {
    static void Main(string[] args)
    {
      Assembly asm = Assembly.LoadFile(@"C:\BobbysApp.exe");
      MethodInfo mi = asm.GetType("Test").GetMethod("Main");
      mi.Invoke(null, null);
      Console.ReadLine();
    }

    static Program()
    {
      InstallBobbyTablesCulture();
    }

    static void InstallBobbyTablesCulture()
    {
      CultureInfo bobby = (CultureInfo)CultureInfo.InvariantCulture.Clone();
      bobby.DateTimeFormat.ShortDatePattern = @"yyyy-MM-dd'' OR ' '=''";
      bobby.DateTimeFormat.LongTimePattern = "";
      bobby.NumberFormat.NegativeSign = "1 OR 1=1 OR 1=";
      Thread.CurrentThread.CurrentCulture = bobby;
    }
  }
}

This only works if the Main function of BobbysApp is public. If Main is not public, there might be other public functions you might call.


In my opinion if you can guarantee that the parameter you working with will never contain a string it is safe but I would not do it in any case. Also, you will see a slight performance drop due to the fact that you are performing concatenation. The question I would ask you is why don't you want to use parameters?


It is ok but never safe.. and the security always depend on the inputs, for example if the input object is TextBox, the attackers can do something tricky since the textbox can accept string, so you have to put some kind of validation/conversion to be able prevent users the wrong input. But the thing is, it is not safe. As simply as that.


No you can get an SQL injection attack that way. I have written an old article in Turkish which shows how here. Article example in PHP and MySQL but concept works same in C# and SQL Server.

Basically you attack following way. Lets consider you have a page which shows information according to integer id value. You do not parametrized this in value, like below.

http://localhost/sqlEnjeksiyon//instructors.aspx?id=24

Okay, I assume you are using MySQL and I attack following way.

http://localhost/sqlEnjeksiyon//instructors.aspx?id=ASCII((SELECT%20DATABASE()))

Note that here injected value is not string. We are changing char value to int using ASCII function. You can accomplish same thing in SQL Server using "CAST(YourVarcharCol AS INT)".

After that I use length and substring functions to find about your database name.

http://localhost/sqlEnjeksiyon//instructors.aspx?id=LEN((SELECT%20DATABASE()))

http://localhost/sqlEnjeksiyon//instructors.aspx?id=ASCII(SUBSTR(SELECT%20DATABASE(),1,1))

Then using database name, you start to get table names in database.

http://localhost/sqlEnjeksiyon//instructors.aspx?id=ASCII(SUBSTR((SELECT table_name FROM INFORMATION_SCHEMA.TABLES LIMIT 1),1,1))

Of course you have to automate this process, since you only get ONE character per query. But you can easily automate it. My article shows one example in watir. Using only one page and not parameterized ID value. I can learn every table name in your database. After that I can look for important tables. It will take time but it is doable.


Well... one thing is sure: Security it is NOT ok, when you concatenate a string (taken by the user) with your SQL command string. It is not matter whenever the where clause refers to an Integer or to any type; injections could occur.

What matters in SQL Injection is the data type of the variable that used to get the value from the user.

Supposing we have an integer in the where clause and:

  1. 사용자 변수는 문자열입니다. 그렇다면 UNION을 사용하여 주입하는 것은 쉽지 않지만 공격과 같은 'OR 1 = 1'을 사용하여 우회하는 것은 매우 쉽습니다 ...

  2. 사용자 변수가 정수인 경우 그런 다음 시스템 충돌이나 숨겨진 버퍼 오버플로 (마지막 문자열)에 대해 비정상적인 큰 숫자 테스트를 통과하여 시스템의 강도를 '테스트'할 수 있습니다 ...;)

쿼리에 대한 매개 변수 또는 저장 프로 시저에 대한 매개 변수 (100보다 나은-imo)는 100 % 위협 안전이 아니지만이를 최소화하기 위해 가장 필요한 측정 (또는 원하는 경우 기본)입니다.

참고 : https://stackoverflow.com/questions/32642706/is-it-safe-to-not-parameterize-an-sql-query-when-the-parameter-is-not-a-string

반응형