Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update: SQL_Injection_Prevention_Cheat_Sheet - SQL Injection #1201

Open
rsrinivasanhome opened this issue Sep 13, 2023 · 7 comments
Open

Update: SQL_Injection_Prevention_Cheat_Sheet - SQL Injection #1201

rsrinivasanhome opened this issue Sep 13, 2023 · 7 comments
Assignees
Labels
ACK_OBTAINED Issue acknowledged from core team so work can be done to fix it. UPDATE_CS Issue about the update/refactoring of a existing cheat sheet.

Comments

@rsrinivasanhome
Copy link

rsrinivasanhome commented Sep 13, 2023

https://cheatsheetseries.owasp.org/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.html

What is missing or needs to be updated?

(https://cheatsheetseries.owasp.org/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.html#defense-option-4-escaping-all-user-supplied-input)

Defense Option1 and Defense Option 2 are not enough to prevent SQL injection . In addition to option 1 or option 2 option 4 is required.

Defense Option 4: Escaping All User-Supplied Input[¶]
This technique should only be used as a last resort, when none of the above are feasible. Input validation is probably a better choice as this methodology is frail compared to other defenses and we cannot guarantee it will prevent all SQL Injections in all situations.

How should this be resolved?

Defense Option 4: Escaping All User-Supplied Input
This technique must be used along with Option 1 and Option 2 . Input validation is probably a better choice as this methodology is frail compared to other defenses and we cannot guarantee it will prevent all SQL Injections in all situations however it may not always be practically feasible as end user(s) often require special characters

We need to explicitly mention Option 4 is also required as part of Option 1 and Option 2.

We have followed the usage of parametrized queries or sql stored procedure and still found application open to SQL injection related attacks. Here is an example how

SQL Server sample - We had also seen a similar issue with postgress also

create table test(name nvarchar(2000))

create procedure proc_test (@p nvarchar(2000))
as
begin
update test
set name = @p
end

 

declare @p1 nvarchar(2000)
set @p1= ''+@@version+''
exec proc_test @p1

select * from test

In the sample above the sql version gets stored in column test can be retrieved .

@rsrinivasanhome rsrinivasanhome added ACK_WAITING Issue waiting acknowledgement from core team before to start the work to fix it. HELP_WANTED Issue for which help is wanted to do the job. UPDATE_CS Issue about the update/refactoring of a existing cheat sheet. labels Sep 13, 2023
@szh
Copy link
Collaborator

szh commented Sep 13, 2023

Yes you are correct. Do you want to create a PR to address this?

@szh szh added ACK_OBTAINED Issue acknowledged from core team so work can be done to fix it. and removed ACK_WAITING Issue waiting acknowledgement from core team before to start the work to fix it. labels Sep 13, 2023
@rsrinivasanhome
Copy link
Author

@szh I will be happy to work on the PR

@szh szh removed the HELP_WANTED Issue for which help is wanted to do the job. label Sep 13, 2023
@mackowski
Copy link
Collaborator

@rsrinivasanhome do you need any help on that?

@rsrinivasanhome
Copy link
Author

Later during further analysis I have out while performing
Option 1: Use of Prepared Statements (with Parameterized Queries)
Option 2: Use of Properly Constructed Stored Procedures
The client libraries internally perform Option 4: Escaping user supplied input.
So when the code above is integrated with C# it will not be susceptible to SQL injection as the client libraries escape the user input . So no longer propose a change.

Let me know your thoughts.

@jmanico
Copy link
Member

jmanico commented Nov 24, 2023

I just submitted a major re-write of the SQL Injection cheatsheet where we heavily discouraged manual escaping. Can you review that please?

@rsrinivasanhome
Copy link
Author

I would be able to review . You can point me to the pull request

@szh
Copy link
Collaborator

szh commented Nov 27, 2023

@rsrinivasanhome #1228

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ACK_OBTAINED Issue acknowledged from core team so work can be done to fix it. UPDATE_CS Issue about the update/refactoring of a existing cheat sheet.
Projects
None yet
Development

No branches or pull requests

4 participants