Solved

Am I protect against SQL Injection?

Posted on 2008-10-30
7
151 Views
Last Modified: 2010-03-20
Hello,

I have read several articles on MSDN, here at Experts-Exchange, etc. to better understand SQL injection and prevention standards.  Below is a result of this research.  However, I would appreciate it if someone could confirmed that this is the path I need to take.

Thank you in advance.
CREATE PROCEDURE [dbo].[sp_addNavElement]
(
	@navID varchar(30), 
	@navURL varchar(30)
)
 
AS 
 
DECLARE @cmd nvarchar(max)
DECLARE @parameters nvarchar(max)
 
SET @cmd = N'INSERT INTO navSystemA (navID, navURL, navOrder) Values (@navID, @navURL, (select count(*) from navSystemA)+ 1 )'
 
SET @parameters ='@navID varchar(30), @navURL varchar(30)'
EXEC sp_executesql @cmd, @parameters, @navID, @navURL

Open in new window

0
Comment
Question by:trumpman
[X]
Welcome to Experts Exchange

Add your voice to the tech community where 5M+ people just like you are talking about what matters.

  • Help others & share knowledge
  • Earn cash & points
  • Learn & ask questions
  • 2
  • 2
  • 2
  • +1
7 Comments
 
LVL 70

Expert Comment

by:Éric Moreau
ID: 22849594
it would be a lot safer to use a Stored Procedure instead of a dynamic query. But since you are limiting the lenght to 30 characters, not much can be done.
0
 

Author Comment

by:trumpman
ID: 22849941
@emoreau:

Perhaps I am missing something?  This is a Stored Procedure.
0
 
LVL 70

Expert Comment

by:Éric Moreau
ID: 22850108
No your not missing anything. I was! As long as you are calling this SP using Command objects, you should not have problems.
0
PeopleSoft Has Never Been Easier

PeopleSoft Adoption Made Smooth & Simple!

On-The-Job Training Is made Intuitive & Easy With WalkMe's On-Screen Guidance Tool.  Claim Your Free WalkMe Account Now

 
LVL 22

Expert Comment

by:dportas
ID: 22854276
This is a poor example because the dynamic SQL is completely unnecessary - it would be better just to put the INSERT in the proc without EXEC or sp_executesql. Either way, it isn't vulnerable to SQL injection - it's just inefficient and poor practice.

PS. Never use the "sp_" prefix for user procs. sp_ is reserved for system procs and will adversely affect your code.
0
 

Author Comment

by:trumpman
ID: 22855740
@dportas:

Could you elaborate on your suggested method?  .."better just to put the INSERT in the proc without EXEC or sp_executesql"...

Thank you.  How would the desired result look?

- Trumpman
0
 
LVL 22

Accepted Solution

by:
dportas earned 260 total points
ID: 22856084
CREATE PROCEDURE [dbo].[prc_addNavElement]
(
      @navID VARCHAR(30),
      @navURL VARCHAR(30)
)
AS
BEGIN;
      INSERT INTO navSystemA (navID, navURL, navOrder)
      VALUES (@navID, @navURL,(SELECT COUNT(*) FROM navSystemA)+ 1 );
END;
0
 
LVL 2

Assisted Solution

by:devshb
devshb earned 240 total points
ID: 22856308
In theory I think you're still open to xss attacks, so by using the logic you've got I think you're pretty much covered against direct injection, but people could still use script tags etc, so you might also want to clean the value of any < or > characters etc during the insert, although it depends on the nature of the data.

See also the "resources" section on the site:
http://www.sqlinjectionscanner.com/
which explains the difference between xss attacks and direct injection
(and that site also has a free data scanner)
0

Featured Post

Optimizing Cloud Backup for Low Bandwidth

With cloud storage prices going down a growing number of SMBs start to use it for backup storage. Unfortunately, business data volume rarely fits the average Internet speed. This article provides an overview of main Internet speed challenges and reveals backup best practices.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

This is basically a blog post I wrote recently. I've found that SARGability is poorly understood, and since many people don't read blogs, I figured I'd post it here as an article. SARGable is an adjective in SQL that means that an item can be fou…
In this article I will describe the Backup & Restore method as one possible migration process and I will add the extra tasks needed for an upgrade when and where is applied so it will cover all.
There are cases when e.g. an IT administrator wants to have full access and view into selected mailboxes on Exchange server, directly from his own email account in Outlook or Outlook Web Access. This proves useful when for example administrator want…
NetCrunch network monitor is a highly extensive platform for network monitoring and alert generation. In this video you'll see a live demo of NetCrunch with most notable features explained in a walk-through manner. You'll also get to know the philos…

690 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question