How to execute dynamic sql statement

Question

Post reply

How to execute dynamic sql statement

mcfarlandparkway

SSCertifiable

Points: 7699
More actions
August 4, 2017 at 12:30 pm

#324833

ALTER PROCEDURE dbo.Employee
(
@IdList NVARCHAR(3500)
)
DECLARE
@SqlStatement NVARCHAR(4000) =
  N'
   INSERT INTO #Employee
   (
    EmployeeId
   )
   SELECT
   EmployeeId
   FROM
   dbo.Employee
   WHERE
   EmployeeId IN (' + @IdList + N');
  ';
IF ( OBJECT_ID('tempdb..#Employee') IS NOT NULL )
  DROP TABLE #Employee;
CREATE TABLE #Employee
(
  EmployeeId INTEGER NOT NULL
)
EXEC ( @SqlStatement )
DROP TABLE #Employee;
RETURN;
END

Viewing 6 posts - 1 through 5 (of 5 total)

You must be logged in to reply to this topic. Login to reply

Thom A SSC Guru Points: 99133 More actions · Answer 1

What is your question here exactly?

Thom~

Excuse my typos and sometimes awful grammar. My fingers work faster than my brain does.
Larnu.uk

Thom A SSC Guru Points: 99133 More actions · Answer 2

Also, that SP wide open for injection.

Edit: Ok, honestly, that entire SQL is a mess. You have a table [Employee] and a SP called [Employee] You can't do that, hyou can't have two objects with the same name.What are you trying.

Then, as I said before, This is WIDE open for injection. Renaming the SP to this (as again, you can't have 2 objects of the same name), the following would inject a create statement into your database and CREATE a table:
EXEC Employee_sp '''''); CREATE TABLE Injection (ID int);--';

Thom~

Excuse my typos and sometimes awful grammar. My fingers work faster than my brain does.
Larnu.uk

Phil Parkin SSC Guru Points: 246985 More actions · Answer 3

There is no need for dynamic SQL here. Instead, create a temp table & put the contents of IdList into it. From there, you can do what you need. This should solve the injection problem too.

Also, the proc seems to follow alien logic:
* Create temp table
* Put some Ids in it
* Drop the temp table
What's the point of doing this?

Finally, proc names should generally not be nouns – because a noun by itself does not tell you what the proc is doing.
Better to use a verb-noun combination – InsertEmployee, DeleteEmployee etc etc.

The absence of evidence is not evidence of absence.
Martin Rees

You can lead a horse to water, but a pencil must be lead.
Stan Laurel

mcfarlandparkway SSCertifiable Points: 7699 More actions · Answer 4

This procedure is used for asp.net front end.
In web page Based on the selection of Employee id's .. those id's should pass to this stored procedure.

In the front end the Id's will generate in this format 475675,857479,487578
with comma seperated ( All Id's will join with (,))

SO these Id's i am trying to pass sql server database stored procedure
Is there other way to write this procedure to avoid sql injection?

The idea of this procedure is what ever we selected in the front end the employee id's and drop down selection start date and end date has to be passed to this procedure, So i am thinking we can create parameters in this procedure, As of now in the below procedure i have only @Idlist as parameter.how i can filter with start and end date?

@startdate
@enddate
@IdList

---Could any one give me the example what is the best way.

ALTER PROCEDURE dbo.Employee
(
@IdList NVARCHAR(3500)
)

DECLARE
@SqlStatement NVARCHAR(4000) =
N'
INSERT INTO #Employee
(
EmployeeId
)
SELECT
EmployeeId
FROM
dbo.Employee
WHERE
EmployeeId IN (' + @IdList + N');
';

IF ( OBJECT_ID('tempdb..#Employee') IS NOT NULL )
DROP TABLE #Employee;

CREATE TABLE #Employee
(
EmployeeId INTEGER NOT NULL
)

EXEC ( @SqlStatement )

DROP TABLE #Employee;

RETURN;
END

Phil Parkin SSC Guru Points: 246985 More actions · Answer 5

My previous post (just above yours) asks some questions and proposes an alternative method which avoids injection.
Perhaps you missed it? Please check again.

The absence of evidence is not evidence of absence.
Martin Rees

You can lead a horse to water, but a pencil must be lead.
Stan Laurel