|
This is very similar to a previous post but with different code.
I have to eliminate a SQL injection error from within a method. Now, with only minor modifications this error must be eliminated. Here is the description from the scan:
Attack vector: system_data.system.data.IDbCommand.ExecuteReader
Description: The database query contains a sql injection flaw. The call to system_data_dll.System.Data.IDbCommand.ExecuteReader constructs a dynamic sql query using a variable derived from user-supplied input. An attacker could exploit this flaw to execute arbitrary sql queries against the database. ExecuteReader was called on the command object, which contains tainted data. The tainted data originated from earlier calls to system_data_dll.data.common.dbcommand.executereader, System_web_dll.system.web.httprequest.get_params, system_web_dll.data.common.dbadapter_fill, system_data_dll.system.data.common.dbwommand.executescarar and system_web_dll.system.web.httprequest.get_form
Code:
protected DataTable ExecuteDataTable(DbCommand command, ParamData[] pDataArr)
{
DataTable returnValue = null;
try
{
if (_connection == null)
OpenConnection();
else
{
if (_connection.State == ConnectionState.Closed)
OpenConnection();
}
command.Connection = _connection;
command.CommandType = CommandType.Text;
command.CommandTimeout = 12000;
for (int i = 0; i < pDataArr.Length; i++)
{
DbParameter parameter = command.CreateParameter();
parameter.ParameterName = pDataArr[i].pName;
parameter.DbType = pDataArr[i].pDataType;
parameter.Value = pDataArr[i].pValue;
command.Parameters.Add(parameter);
}
returnValue = new DataTable();
DbDataReader reader;
reader = command.ExecuteReader();
using (reader)
{
returnValue.Load(reader, LoadOption.OverwriteChanges);
}
reader.Close();
if (!KeepAlive && _connection.State == ConnectionState.Open)
{
CloseConnection();
}
}
catch (Exception e)
{
if (e is EntryPointNotFoundException)
throw e;
_iserror = true;
LogBLL bll = new LogBLL();
bll.WriteErrorLog(e);
}
pDataArr = null;
return returnValue;
}
Thanks in advance!
modified 12-May-15 17:16pm.
|
|
|
|
|
I assume that it's the same thing as in your previous question. Though there are SQL-parameters used in this method, it gets its command-object passed as an argument with the command-text apparently already assigned. I guess the calling code concatenates some values (other than there are in pDataArr) as literals into the query string.
If the brain were so simple we could understand it, we would be so simple we couldn't. — Lyall Watson
|
|
|
|
|
I saw a very good answer to my question about parameterizing a string with the actual parameters; how would I know whether the command was an UPDATE, INSERT, or a DELETE?
SqlCommand cmd = new SqlCommand(commandText, connection);
SqlParameterCollection sp = cmd.Parameters;
List<SqlParameter> sp = new List<SqlParameter>()
{
new SqlParameter() {ParameterName = "@CmpyCode", SqlDbType = SqlDbType.NVarChar, Value= CV.Global.CMPYCODE},
new SqlParameter() {ParameterName = "@Code", SqlDbType = SqlDbType.NVarChar, Value = codeName},
new SqlParameter() {ParameterName = "@DisplayCode", SqlDbType = SqlDbType.NVarChar, Value = codeName + "-"},
new SqlParameter() {ParameterName = "@TotalDigit", SqlDbType = SqlDbType.Int, Value = CV.Global.PARAMTOTALDIGIT}
};
insertData(CV.Sps.SP_INSERT_PARAM_TABLE, sp);
SqlCommand cmd = new SqlCommand();
cmd.Parameters.AddRange(parameterPasses.ToArray());
|
|
|
|
|
Steve Holdorf wrote: how would I know whether the command was an UPDATE, INSERT, or a DELETE? I'm a bit confused - what is the context for this question? I don't see how it is related to your previous questions. And I don't see why you posted that code, which appears to be three separate fragments?
If the brain were so simple we could understand it, we would be so simple we couldn't. — Lyall Watson
|
|
|
|
|
OK. Let me explain. I have found some code that I think will work but how would I know what sql command it would be. See code below:
SqlCommand cmd = new SqlCommand(commandText, connection);
SqlParameterCollection sp = cmd.Parameters;
List<SqlParameter> sp = new List<SqlParameter>()
{
new SqlParameter() {ParameterName = "@CmpyCode", SqlDbType = SqlDbType.NVarChar, Value= CV.Global.CMPYCODE},
new SqlParameter() {ParameterName = "@Code", SqlDbType = SqlDbType.NVarChar, Value = codeName},
new SqlParameter() {ParameterName = "@DisplayCode", SqlDbType = SqlDbType.NVarChar, Value = codeName + "-"},
new SqlParameter() {ParameterName = "@TotalDigit", SqlDbType = SqlDbType.Int, Value = CV.Global.PARAMTOTALDIGIT}
};
insertData(CV.Sps.SP_INSERT_PARAM_TABLE, sp);
SqlCommand cmd = new SqlCommand();
cmd.Parameters.AddRange(parameterPasses.ToArray());
|
|
|
|
|
You didn't explain a whole lot
Am I right in assuming that you want to change your code to use stored procedures instead of 'text-statements' (CommandType.Text) and in order to execute the right one, need to know if it should be an UPDATE, INSERT, or DELETE ?
If the brain were so simple we could understand it, we would be so simple we couldn't. — Lyall Watson
|
|
|
|
|
No. I still have to use the static command but eliminate the hard coded parameters. That being the case I want to use the technique above for passing in the parameters, use the loop, which is the problem to begin with, to fill in a parameterized string like so:
command.CommandText = ("INSERT INTO TABLE (result, title, des) values(@store_result, @store_title, @store_des)");
-- modified 12-May-15 20:39pm.
|
|
|
|
|
I still can't follow you completely. You're omitting some steps of your train of thought
The method in your original post takes a DbCommand and ParamData[], already has some kind of parameter-filling-loop and then runs an ExecuteReader(). The DbCommand apparently already has a SELECT-statement assigned and my assumption was that this statement already contains some values as literals, which is why the method failed your SQL-injection test, despite the rest of the values are added via parameters.
As you're now quoting an INSERT-statement, you must be talking about some completely different method that I've not seen yet. Please post that method and also the calling code and maybe elaborate on why using a loop to create the parameters is a problem.
If the brain were so simple we could understand it, we would be so simple we couldn't. — Lyall Watson
|
|
|
|
|
Lefevre,
You make a good point about me being confused. I really can't show you anything that doesn't require a loop. I have three other findings that are simple hard coded commands with parameters as part of the string. This is what I showed you in my first post that you answered before this one. In every case a loop is required to add the sql parameters to the command. I have been looking for some kind of Lambda expression to add the values to the sql command parameter list which I can not find.
|
|
|
|
|
How about we deal first with the method you posted in your original question?
To help you further with that, I would like to see the calling code - can you post that?
/Sascha
If the brain were so simple we could understand it, we would be so simple we couldn't. — Lyall Watson
|
|
|
|
|
I think I have a solution. Can someone review the solution and let me know what they think?
The Singleton created with it's _id property that is passed in from the calling function:
public class QueryContainer
{
private static List<querycontainer> Container;
private static QueryContainer instance;
private int _id;
public int _searchID;
private string _query;
private QueryContainer () { }
public static QueryContainer Instance
{
get {
if (Instance == null)
{
instance = new QueryContainer();
}
return instance;
}
}
public string Query { get { return Container.Find(instance => instance._id == _searchID).Query; }
set { Container.Query = value; _id =+ 1; } }
}
}
}
public int ID { get { return _id; } }
}
The calling code that passes the id to access the query string from the singleton:
protected object ExecuteScaler(int id)
{
object returnValue = null;
Container Instance = new Container ();
Instance.searchID = id;
DbCommand command = _provider.CreateCommand();
command.Connection = _connection;
command.CommandText = Instance.Query;
command.CommandType = CommandType.Text;
if (_useTransaction) { command.Transaction = _transaction; }
try
{
returnValue = command.ExecuteScalar();
}
...
|
|
|
|
|
It's awful. It really is, sorry.
- A singleton is completely unneccessary for what you're trying to do.
- It's the worst possible implementation of the singleton pattern.
- It won't compile because of the setter of the Query-property.
- The getter of the Query-property will cause a stack overflow.
- There's no need for a "Query-Container-List" for what you're trying to do.
- It doesn't address the actual problem of SQL-injection / SQL-parameters at all.
I suggested a way for further course of action in my last message - it's up to you
If the brain were so simple we could understand it, we would be so simple we couldn't. — Lyall Watson
|
|
|
|
|
There is no substitute for doing it the right way.
|
|
|
|
|
I have to eliminate a SQL injection error from within a method. What the code is doing is passing in a SQL querystring as the command to a DbCommand object, see the code below. Now, with only minor modifications this error must be eliminated. Here is the description from the scan:
This database query contains a sql injection flaw. the call to system_data_dll.Data.IDbCommand.ExecuteNonQuery constructs a dynamic sql queryusing a variable derived from the user-supplied input.An attacker could exploit this flaw to execute arbitrary sql queries against the database ExecuteNonQuery was called on the command object, which contains tainted data. The tainted data originated from from earlier calls to system_data.system.data.common..dbconnand.execurereader, system_web_dll.wweb.httprequest.get_params, system_data_dll.system.data.system.data.common.dbaadapter.fill.
Below is the actual function code:
protected object ExecuteScaler(string queryString)
{
object returnValue = null;
if (!_iserror)
{
if (_trace)
{ DoTrace("TAMIS.Data.Loader.ExecuteScalar", queryString); }
if (_connection == null || _connection.State == ConnectionState.Closed)
{
OpenConnection();
}
DbCommand command = _provider.CreateCommand();
command.Connection = _connection;
command.CommandText = queryString;
command.CommandType = CommandType.Text;
if (_useTransaction) { command.Transaction = _transaction; }
try
{
returnValue = command.ExecuteScalar();
}
catch (Exception ex)
{
if (ex is EntryPointNotFoundException)
throw ex;
//if (_useTransaction == true)
//_transaction.Rollback();
RollBack();
LogBLL bll = new LogBLL();
bll.WriteErrorLog(ex);
_iserror = true;
}
finally
{
if ((!KeepAlive && _connection.State == ConnectionState.Open) || _iserror == true)
{
CloseConnection();
}
}
}
else
{
returnValue = -1;
}
return returnValue;
}
Thanks in advance for all of your help!
|
|
|
|
|
The way to avoid conventional SQL-injection attacks is to use SQL-parameters. That is, not passing the values for your query as literals concatenated into the SQL-statement but to have the names of SQL-parameters in those places of the SQL-statement instead and add SQL-parameters 'carrying' the actual values to the parameter-collection of the command-object.
E.g. instead of this:
SELECT col1 FROM table1 WHERE col2 = 'string from user input';
..it should be like this:
SELECT col1 FROM table1 WHERE col2 = @userinput;
..plus creating an SQL-Parameter with a name of "@userinput", assigning the value 'string from user input' to it and adding it to the parameter-collection of the command-object.
But the posted method is being passed the final query string as an argument, so it's already too late to do this. You will have to change this method
- either to accept an SQL-parameter-collection with the parameters created where the method is being called
- or to accept a value-collection and create SQL-parameters for those values
and then add the SQL-parameters to the command-object. And the calling code obviously has also to be changed accordingly.
http://www.dotnetperls.com/sqlparameter[^]
https://msdn.microsoft.com/en-us/library/system.data.common.dbparameter%28v=vs.110%29.aspx[^]
If the brain were so simple we could understand it, we would be so simple we couldn't. — Lyall Watson
|
|
|
|
|
holdorf wrote: Now, with only minor modifications this error must be eliminated.
You won't be able to solve this problem with only minor modifications. You'll need to change your data access methods to accept parameters, and change every bit of code that calls them to pass parameters instead of using string concatenation. Since your code seems to be intended to work with multiple database systems, you'll also need to find a way to use the correct parameter representation for each provider - for example, SqlCommand uses named parameters, but OleDbCommand uses positional parameters.
Since you need to fundamentally change your code anyway, you might want to consider replacing your custom data-access methods with something like Dapper[^]. That way, you can concentrate on fixing the code that calls your data access methods, instead of fixing the data access methods themselves.
In case you need it, Troy Hunt has an excellent introductory explanation of SQL Injection on his blog:
Everything you wanted to know about SQL injection (but were afraid to ask) [^]
"These people looked deep within my soul and assigned me a number based on the order in which I joined."
- Homer
|
|
|
|
|
I know there are a lot of posts but I finally understand and did what I was told to do. I broke the query up with parameters and I am still getting the security error. My code is below the with the parameters removed from the hard coded string, the calling code, and the implementing code:
The 3 classes with the SQL w/ with the parameters broken out, the calling code, and the implementing code:
Class with the parameters broken out:
public class MyParam
{
public string name { get; set; }
public string value { get; set; }
}
public class QueryContainer
{
string _query;
public List parameterList = new List();
public QueryContainer(string query) { _query = query; }
public string Query
{
get
{
return _query;
}
set { _query = value; }
}
}<pre>
The calling code:
<pre>
public int GetAccountSortByAccountCode(int account)
{
QueryContainer Instance = new QueryContainer("SELECT ac_sort_order FROM lkup_account_codes where ac_code = <a href="http:
MyParam myParam = new MyParam();
myParam.name = "@account";
myParam.value = account.ToString();
Instance.parameterList.Add(myParam);
return Convert.ToInt32(ExecuteScaler(Instance, 1));
}
<pre>
The implementing code:
<pre>
if (_connection == null || _connection.State == ConnectionState.Closed)
{
OpenConnection();
}
DbCommand command = _provider.CreateCommand();
command.Connection = _connection;
{
command.CommandText = Instance.Query;
command.CommandType = CommandType.Text;
foreach (var p in Instance.parameterList)
{
SqlParameter param = new SqlParameter(p.name, p.value);
command.Parameters.Add(param);
}
if (_useTransaction) { command.Transaction = _transaction; }
try
{
returnValue = command.ExecuteScalar();
}
catch (Exception ex)
{
if (ex is EntryPointNotFoundException)
throw ex;
RollBack();
LogBLL bll = new LogBLL();
bll.WriteErrorLog(ex);
_iserror = true;
}
<pre>
|
|
|
|
|
I'm sort of fuzzy about how the sequence of operation works here on this.
I wrote the code, and now I'm building the pieces for it like the web page and buttons.
Here's how I think it works, if I'm totally wrong let me know.
- Write the XML file to submit to Authorize.net, for a PayPal transaction: Sort of letting Authorize.net or PayPal know that a transaction is coming their way.
- Transmit the XML file to the endpoint at Authorize.Net: I think I'm suppose to pickup the response XML and feed some data to the step 3
- Redirect the customer to the PayPal Website, using the PayPal Account Settings and URL plus querystrings
4, Then the customer goes through the motions of paying
- The customer comes back to the success or cancel page.
- I get a response in some format, with a token or transaction ID and result code.
|
|
|
|
|
Hello,
Can anyone suggest me some of best Free dashboard samples?
I mostly needed webbased type either ASP.NET or HTML type
Regards,
SMA
|
|
|
|
|
Google will help, search for free dashboard templates you can find many for free.
modified 20-Sep-20 21:01pm.
|
|
|
|
|
i have project buil web with Web Accessibility...plz i dont undertand web accessibility..plz share me some book ...
|
|
|
|
|
|
W3 Org would define it best!
https://www.w3.org/WAI/intro/accessibility.php[^]
Definition is: Web accessibility means that people with disabilities can use the Web. You can use Google to search for more on this topic.
The sh*t I complain about
It's like there ain't a cloud in the sky and it's raining out - Eminem
~! Firewall !~
|
|
|
|
|
I'm stuck with this problem, any help will be appreciated.
I have a web service, it requires authentication to call functions but I couldn't authenticate.
The function XML is like this:
<s11:Envelope xmlns:s11='http://schemas.xmlsoap.org/soap/envelope/'>
<s11:Header>
<ns1:AuthHeader xmlns:ns1='MOD'>
<ns1:Channel>?XXX?</ns1:Channel>
<ns1:Username>?XXX?</ns1:Username>
<ns1:Password>?XXX?</ns1:Password>
</ns1:AuthHeader>
</s11:Header>
<s11:Body>
<ns1:GetBankList xmlns:ns1='MOD' />
</s11:Body>
</s11:Envelope>
my connection codes are:
$baglanti = new :confused:SoapClient("https://galaksi.turknippon.com/appservice/mod.asmx?wsdl");
$parm = array();
$parm[] = new SoapVar('channelcode', XSD_STRING, null, null, 'Channel' );
$parm[] = new SoapVar('myusername', XSD_STRING, null, null, 'Username' );
$parm[] = new SoapVar('mypassword', XSD_STRING, null, null, 'Password' );
$Adres = "https://galaksi.turknippon.com/appservice/";
$Baslik = new SoapHeader($Adres, "AuthHeader",new SoapVar($parm, SOAP_ENC_OBJECT));
$baglanti->__setSoapHeaders(array($Baslik));
$sonuc = $baglanti->AuthHeader(new SoapVar($parm, SOAP_ENC_OBJECT));
print_r ($sonuc);
Here is a stack trace:
System.Web.Services.Protocols.SoapException: Server was unable to process request. ---> System.NullReferenceException: Object reference not set to an instance of an object. at AppServiceLibrary.AuthHeader.Validate(AuthHeader& credential) in xxxx\Galaxy\Galaxy\AppServiceLibrary\AuthHeader.cs:line 132 at xxxx.AppService.MODService.GetBankList() in xxxx\AppService\mod.asmx.cs:line 1148 --- End of inner exception stack trace --- –
|
|
|
|
|
What is the error that you get?
There are only 10 types of people in the world, those who understand binary and those who don't.
|
|
|
|
|