c++ - Is using ODBC {CALL} to a SQL Server stored-procedure safe from SQL injection? - Stack Overflow

admin管理员组

文章数量:1287790

I am responsible for securing an old Windows C++ project. It uses the MFC ODBC CDatabase class to interface with SQL-Server. All DB query and insert operations are done using parameterized stored-procedures, so it seems like good practice.

But almost all the DB C++ code prepares statements in this form:

//ParamStr1 and ParamStr2 are string-type API parameters
CString SQL;
CDatabase DB;
SQL.Format("{CALL InsertStoredProcedureXYZ('%s','%s')}", ParamStr1, ParamStr2);
DB.ExecuteSQL(SQL);

A similar thing is done for queries:

//ParamStr1 and ParamStr2 are string-type API parameters
CString SQL;
CRecordset Recordset;
SQL.Format("{CALL QueryStoredProcedure('%s','%s')}", ParamStr1, ParamStr2);
Recordset.Open(CRecordset::forwardOnly, SQL, CRecordset::readOnly);

My worry is that the parameters used for these statements often come directly from web API parameters. The code does some simple parameter screening, mainly to replace ' with '' and to verify string length.

I believe {CALL SP()} is an ODBC thing. I don't think SQL-Server has a CALL command. So is ODBC doing injection-safe parameter preparation before invoking the stored-procedure? I also know that ODBC can instead be used to call stored procedures using {CALL SP(?,?)} followed by parameter-binding. That seems to be better practice but changing to this would require a huge refactor of the many times that the project code prepares CALL statements.

This C++ MFC ODBC code seems quite old fashioned so I have failed to find anything that definitively tells me whether or not I can trust building {CALL SP()} strings directly from API parameters. Is this just bad old SQL statement building disguised as good practice?

I am responsible for securing an old Windows C++ project. It uses the MFC ODBC CDatabase class to interface with SQL-Server. All DB query and insert operations are done using parameterized stored-procedures, so it seems like good practice.

But almost all the DB C++ code prepares statements in this form:

//ParamStr1 and ParamStr2 are string-type API parameters
CString SQL;
CDatabase DB;
SQL.Format("{CALL InsertStoredProcedureXYZ('%s','%s')}", ParamStr1, ParamStr2);
DB.ExecuteSQL(SQL);

A similar thing is done for queries:

//ParamStr1 and ParamStr2 are string-type API parameters
CString SQL;
CRecordset Recordset;
SQL.Format("{CALL QueryStoredProcedure('%s','%s')}", ParamStr1, ParamStr2);
Recordset.Open(CRecordset::forwardOnly, SQL, CRecordset::readOnly);

My worry is that the parameters used for these statements often come directly from web API parameters. The code does some simple parameter screening, mainly to replace ' with '' and to verify string length.

I believe {CALL SP()} is an ODBC thing. I don't think SQL-Server has a CALL command. So is ODBC doing injection-safe parameter preparation before invoking the stored-procedure? I also know that ODBC can instead be used to call stored procedures using {CALL SP(?,?)} followed by parameter-binding. That seems to be better practice but changing to this would require a huge refactor of the many times that the project code prepares CALL statements.

This C++ MFC ODBC code seems quite old fashioned so I have failed to find anything that definitively tells me whether or not I can trust building {CALL SP()} strings directly from API parameters. Is this just bad old SQL statement building disguised as good practice?

• c++
• sql-server
• windows
• mfc
• odbc

Share Improve this question Follow edited Feb 22 at 23:12 xwellg asked Feb 22 at 20:21 xwellgxwellg 1955 bronze badges 6

• What is SQL? This is not a part of ODBC. – 3CxEZiVlQ Commented Feb 22 at 20:27
• 1 it would take you a couple of minutes to inject some bad parameter strings and see what happens – siggemannen Commented Feb 22 at 21:01
• Use SQL profiler and see exactly what is sent across the wire. – Dale K Commented Feb 22 at 21:37
• The code isn't complete because I just wanted to demonstrate what is being done. SQL is an MFC CString type and Format is a member function that works like sprintf or std::format. – xwellg Commented Feb 22 at 23:04
• I should have mentioned that I have tried to inject SQL into API parameters that go into these CALL statements. Couldn't make anything bad happen but I am no hacker so don't know all the tricks. – xwellg Commented Feb 22 at 23:06

 |  Show 1 more comment

1 Answer 1

Sorted by: Reset to default 2

This is not safe.

We don't see all the code, but since the injected values are surrounded in single quotes, we can conclude that they are being pasted into the SQL batch, rather than being sent as parameters.

SQL.Format("{CALL InsertStoredProcedureXYZ('%s','%s')}", ParamStr1, ParamStr2);

本文标签： cIs using ODBC CALL to a SQL Server storedprocedure safe from SQL injectionStack Overflow