###Error Handling / Execution Flow: Beware of Raptors
Error Handling / Execution Flow: Beware of Raptors
I realize you haven't implemented it yet, but you've set yourself up for some twisted execution flow:
Exit Function 'fixed indentation
ErrorHandler:
EvaluateQuery = ""
End Function
When you do implement error handling, you'll likely be cleaning up resources (closing recordsets, connections, etc.) - you'll want to do that in case of error, and in normal execution paths too.
I'd remove Exit Function and rework the ErrorHandler: part:
ErrorHandler:
'todo: cleanup
If Err.Number <> 0 Then
'todo: handle errors
End If
End Function
This way you'll avoid introducing another label (Cleanup:?) and a GoTo Cleanup, and raptors won't get you.
###SQL Injection
SQL Injection
I know only 1 way of preventing SQL injection: let the server handle the parameters. I don't know what 3rd party you're using nor what a NetRs is (why not use ADODB?), but the code in EvaluateQuery seems to be concatenating the parameters into the query.
This means a String parameter would have to be surrounded by single quotes, and that a string that says "Robert'); DROP TABLE Students;--" would be concatenated - and executed as such. Thus you are correct, this code is screaming SQL injection vulnerability.
Don't even try "sanitizing" the strings before concatenating them into your query. It's the server's job to deal with the parameters. With ADODB you'd be sending the server a query like:
"SELECT SomeField FROM SomeTable WHERE SomeOtherField = ?"
Where ? is replaced by the server, by its parameter value - no need to worry about single quotes around string parameters, the parameter itself has all the information needed for the server to know how to handle.
On top of being more secure, such queries are more performant: if you execute them in a loop, the server receives the same query everytime, only with different parameters - it reuses the same query plan every time and only calculates it once. If you send the parameter values embedded in the query, the server receives a different query every time, and has to recalculate a query plan each time.
See MSDN for more info on parameterized ADODB queries.
###Error Handling / Execution Flow: Beware of Raptors
I realize you haven't implemented it yet, but you've set yourself up for some twisted execution flow:
Exit Function 'fixed indentation
ErrorHandler:
EvaluateQuery = ""
End Function
When you do implement error handling, you'll likely be cleaning up resources (closing recordsets, connections, etc.) - you'll want to do that in case of error, and in normal execution paths too.
I'd remove Exit Function and rework the ErrorHandler: part:
ErrorHandler:
'todo: cleanup
If Err.Number <> 0 Then
'todo: handle errors
End If
End Function
This way you'll avoid introducing another label (Cleanup:?) and a GoTo Cleanup, and raptors won't get you.
###SQL Injection
I know only 1 way of preventing SQL injection: let the server handle the parameters. I don't know what 3rd party you're using nor what a NetRs is (why not use ADODB?), but the code in EvaluateQuery seems to be concatenating the parameters into the query.
This means a String parameter would have to be surrounded by single quotes, and that a string that says "Robert'); DROP TABLE Students;--" would be concatenated - and executed as such. Thus you are correct, this code is screaming SQL injection vulnerability.
Don't even try "sanitizing" the strings before concatenating them into your query. It's the server's job to deal with the parameters. With ADODB you'd be sending the server a query like:
"SELECT SomeField FROM SomeTable WHERE SomeOtherField = ?"
Where ? is replaced by the server, by its parameter value - no need to worry about single quotes around string parameters, the parameter itself has all the information needed for the server to know how to handle.
On top of being more secure, such queries are more performant: if you execute them in a loop, the server receives the same query everytime, only with different parameters - it reuses the same query plan every time and only calculates it once. If you send the parameter values embedded in the query, the server receives a different query every time, and has to recalculate a query plan each time.
See MSDN for more info on parameterized ADODB queries.
Error Handling / Execution Flow: Beware of Raptors
I realize you haven't implemented it yet, but you've set yourself up for some twisted execution flow:
Exit Function 'fixed indentation
ErrorHandler:
EvaluateQuery = ""
End Function
When you do implement error handling, you'll likely be cleaning up resources (closing recordsets, connections, etc.) - you'll want to do that in case of error, and in normal execution paths too.
I'd remove Exit Function and rework the ErrorHandler: part:
ErrorHandler:
'todo: cleanup
If Err.Number <> 0 Then
'todo: handle errors
End If
End Function
This way you'll avoid introducing another label (Cleanup:?) and a GoTo Cleanup, and raptors won't get you.
SQL Injection
I know only 1 way of preventing SQL injection: let the server handle the parameters. I don't know what 3rd party you're using nor what a NetRs is (why not use ADODB?), but the code in EvaluateQuery seems to be concatenating the parameters into the query.
This means a String parameter would have to be surrounded by single quotes, and that a string that says "Robert'); DROP TABLE Students;--" would be concatenated - and executed as such. Thus you are correct, this code is screaming SQL injection vulnerability.
Don't even try "sanitizing" the strings before concatenating them into your query. It's the server's job to deal with the parameters. With ADODB you'd be sending the server a query like:
"SELECT SomeField FROM SomeTable WHERE SomeOtherField = ?"
Where ? is replaced by the server, by its parameter value - no need to worry about single quotes around string parameters, the parameter itself has all the information needed for the server to know how to handle.
On top of being more secure, such queries are more performant: if you execute them in a loop, the server receives the same query everytime, only with different parameters - it reuses the same query plan every time and only calculates it once. If you send the parameter values embedded in the query, the server receives a different query every time, and has to recalculate a query plan each time.
See MSDN for more info on parameterized ADODB queries.
###Error Handling / Execution Flow: Beware of Raptors
I realize you haven't implemented it yet, but you've set yourself up for some twisted execution flow:
Exit Function 'fixed indentation
ErrorHandler:
EvaluateQuery = ""
End Function
When you do implement error handling, you'll likely be cleaning up resources (closing recordsets, connections, etc.) - you'll want to do that in case of error, and in normal execution paths too.
I'd remove Exit Function and rework the ErrorHandler: part:
ErrorHandler:
'todo: cleanup
If Err.Number <> 0 Then
'todo: handle errors
End If
End Function
This way you'll avoid introducing another label (Cleanup:?) and a GoTo Cleanup, and raptors won't get you.
###SQL Injection
I know only 1 way of preventing SQL injection: let the server handle the parameters. I don't know what 3rd party you're using nor what a NetRs is (why not use ADODB?), but the code in EvaluateQuery seems to be concatenating the parameters into the query.
This means a String parameter would have to be surrounded by single quotes, and that a string that says "Robert'); DROP TABLE Students;--" would be concatenated - and executed as such. Thus you are correct, this code is screaming SQL injection vulnerability.
Don't even try "sanitizing" the strings before concatenating them into your query. It's the server's job to deal with the parameters. With ADODB you'd be sending the server a query like:
"SELECT SomeField FROM SomeTable WHERE SomeOtherField = ?"
Where ? is replaced by the server, by its parameter value - no need to worry about single quotes around string parameters, the parameter itself has all the information needed for the server to know how to handle.
On top of being more secure, such queries are more performant: if you execute them in a loop, the server receives the same query everytime, only with different parameters - it reuses the same query plan every time and only calculates it once. If you send the parameter values embedded in the query, the server receives a different query every time, and has to recalculate a query plan each time.
See MSDN for more info on parameterized ADODB queries.