SQL Injection & Sprocs - SQL Server Forums

Please start any new threads on our new site at https://forums.sqlteam.com. We've got lots of great SQL Server experts to answer whatever question you can come up with.

All Forums

SQL Server 2000 Forums

SQL Server Development (2000)

SQL Injection & Sprocs

Author

Topic

RoLYroLLs
Constraint Violating Yak Guru

255 Posts

Posted - 2004-04-21 : 12:32:38

Hi all. I read around several sites learning about SQL Injection and how to try to prevent it. I came across several very similar articles, but none had a definite answer for what I was looking for as why I am here to get some real life human input.

I noticed in many of the articles they mostly speak of dynamically creating an SQL statement at the application level. In my case, however, I do not do this. I simply pass a few parameters through the ado command and get data back, like so:

sproc:

CREATE procedure rolyrolls.proc_GetOption
	@OptionID int,
	@Option varchar(255) output, 
	@OptionLastModified datetime output, 
	@OptionLastModifiedBy int output
as
begin
	select
		@Option = [Option].[Option],
		@OptionLastModified = [Option].OptionLastModified,
		@OptionLastModifiedBy = [Option].OptionLastModifiedBy
	from
		[Option]
	where
		[Option].OptionID = @OptionID
	
	return @@rowcount
end


GO

app code (asp):

		Set cmd = Server.CreateObject("ADODB.Command")
		With cmd
			.ActiveConnection = dbc
			.CommandText = "rolyrolls.proc_GetOption"
			.CommandType = adCmdStoredProc
			
			.Parameters.Append .CreateParameter("@RetVal", adInteger, adParamReturnValue, 4)
			.Parameters.Append .CreateParameter("@OptionID", adInteger, adParamInput, 4, Form_OptionID)
			.Parameters.Append .CreateParameter("@Option", adVarChar, adParamOutput, 255)
			
			.Execute
		End With
		
		If cmd("@RetVal") > 0 Then
			Form_Option = cmd("@Option")
		Else
			Session("error") = "Unexpected error occurred. Please try again."
			Response.Redirect "error.asp"
		End If

Most of my sprocs are similar to this, some may be more complexe but still similar. Also, I always call my sprocs in this matter. I saw somewhere you can call them on one line and include the params as you would write it on QA, which a user could then input a ";" and some devastating command after that. Altho my permissions are well in place, I don't want to fully rely on them and naivly think I'm safe from SQL Injections or anything else. So my question here is am I safe from SQL Injection with my method of coding? If not, why not? And where can I find some documented help for this, since I have failed to find anything concerning sprocs.

Thanks to all in advance.

- RoLY roLLs

AjarnMark
SQL Slashing Gunting Master

3246 Posts

Posted - 2004-04-21 : 17:18:25

Roly, as far as SQL Injection goes, you're in pretty good shape (although I would question the value of having a table and column name be the same: [Option].[Option]). Your only input to the sproc is an integer which makes it really difficult for any SQL statement to be inserted. And that's basically what you're protecting against is a new SQL statement being injected into the middle of your own. When you have more parameters and ones that are varchar, you want to be sure that you keep the size of the parameter only as large as necessary for the data you need. You could also do some input validation such as scrubbing input parameters for keywords like SELECT, INSERT, UPDATE, DELETE, DROP, etc. And if you are not granting any permissions on underlying tables, but only EXECUTE permissions on the sproc you're in better shape yet.

--------------------------------------------------------------
Find more words of wisdom at [url]http://weblogs.sqlteam.com/markc[/url]

RoLYroLLs
Constraint Violating Yak Guru

255 Posts

Posted - 2004-04-21 : 17:41:44

Thanks. As far as the option.option part, i di that because i have about 15 tables with similar stucture as that one and it makes it much eaier to use a replace function on OPTION to the name of the other table and much eaier on the retrieval part as well. If u speak of option because it's a keyword or whatnot, then i guess it wouldn't be very nice, huh?

I do actually have other sp's with varchars, and well while I do some cleanup at the app level b4 sending, i do not check for those keywords u mentioned, partly because they can be words that users can enter, but I'm more worried of the possibility of it being done, I'll lookup way to protect myself later. Most varcahr columns are in the length of 255, with the exception of 3 or 4 of 4000 and another 20 columns in the range of 5 to 20.

So u say I should be worried? Some sprocs do add data to the server so the user needs INSERT and UPDATE permissions, so I assume there's another reason to worry, no?

Thanks.

- RoLY roLLs

Merkin
Funky Drop Bear Fearing SQL Dude!

4970 Posts

Posted - 2004-04-21 : 21:39:01

SQL injection works when you build up a string and execute it, so adding nasty bits to the string will casue them to be executed.

With a stored proc like this (unless you use dynamic sql, which you aren't) you have two levels of protection.

The first is that the batch isn't turned into a full string by the query engine. It is evaluated first, then the param values are inserted. So it already knows what to do before the variables are put in place (very simplified explaination).

The second is that the variables are typed, if you try to put a semicolon after your @OptionID it will never get to the execution stage because "8;" won't cast as an int.

The other thing you can do with a proc is permissions. You can deny INSERT and UPDATE to the user, but give them execute permissions on the proc. That way, they can still run the procedure (which handles your updates) but can't go around it and work directly with the tables. Again, that's a simplified explaination, if you spend some time reading up on permissions in BOL you can get a better idea.

Damian

RoLYroLLs
Constraint Violating Yak Guru

255 Posts

Posted - 2004-04-21 : 22:58:02

OK, that sounds great! One thing I will read up on, but would like an answer from here, is a simple permissions question. I understood from your post, Merkin, that if I remove insert/update permission for, let's say, table1 to a user, but exec on a sproc that adds data to table1, the user can still add, but only thru the sproc? Is that right? If so, WOW! I've missed so much and don't know how. I am big on sprocs and only like to use them to manipulate/view data. So can I remove ALL permission to a user to ALL the tables, but have them interact with the table whether insert/update/delete/select thru the sproc?

EDIT: Sorry Merkin I just reread your post and it answered my question. Guess it's too late for me

- RoLY roLLs

derrickleggett
Pointy Haired Yak DBA

4184 Posts

Posted - 2004-04-21 : 23:03:55

The only exception to this Merkin is cross-database inserts/updates/etc. There are only two ways to deal with this:

1. Set up procedures in the other database that insert/update/select/delete and use these in the procs on other databases.

2. Give access to the tables directly to the user.

Yukon will be addressing this though, thankfully.

MeanOldDBA
derrickleggett@hotmail.com

When life gives you a lemon, fire the DBA.

RoLYroLLs
Constraint Violating Yak Guru

255 Posts

Posted - 2004-04-21 : 23:08:18

Hmm sounds a little out of my league right now

Hehe. Not familiar with cross joins (yet)

- RoLY roLLs

RoLYroLLs
Constraint Violating Yak Guru

255 Posts

Posted - 2004-04-21 : 23:39:00

quote:
Originally posted by Merkin

You can deny INSERT and UPDATE to the user, but give them execute permissions on the proc.

I just tried that and great it updates w/o user permissions on the table. However is it ONLY for INSERT and UPDATE? What about DELETE ro SELECT? I removed SELECT permissions and was not allowed to view, which is fine, I don't mind giving SELECT permissions, but What about DELETE? Works like INSERT and UPDATE or like SELECT? I'm reading permissions on BOL but I'm not finding my answer. I'll keep looking as be waiting for a response. Thanks!

- RoLY roLLs

RoLYroLLs
Constraint Violating Yak Guru

255 Posts

Posted - 2004-04-21 : 23:43:46

Hmm, wierd thing just happened. I removed and I denied all permissions for that table and i WAS able to view the data both times. Guess something else was going on. So with this I assume I can also deny DELETE permissions and be able to delete via sprocs? If so, all your input has been very valuable and greatful! Thanks! Jsut drop by and say YES if that's true. :)

- RoLY roLLs

RoLYroLLs
Constraint Violating Yak Guru

255 Posts

Posted - 2004-04-22 : 13:30:09

Hmm...I'm getting mixed results here. Yesterday I had removed permission for select on a table and was able to view it. Today however, I am not able to.unless i check the select permissions for that table. I get the basic understanding of permissions but not thru sprocs. Any place to find good answers or a list of which permissions works thru sprocs even tho it has been denied for the user...or is it ONLY for INSERT and UPDATE? What about delete?

- RoLY roLLs

tkizer
Almighty SQL Goddess

38200 Posts

Posted - 2004-04-22 : 13:32:26

If you grant EXEC on stored procedures then you do not need to explicitly grant permissions on tables. The user can only do what the stored procedure allows and nothing directly on the tables. In fact, that's one of the benefits of stored procedures, security that is. If you are having problems now due to removing explicit permissions, it is probably because your stored procedures are using dynamic sql. With dynamic sql, you MUST grant explicit permissions on tables.

So which stored procedure are you having problems with and are you using dynamic sql?

Tara

RoLYroLLs
Constraint Violating Yak Guru

255 Posts

Posted - 2004-04-22 : 14:00:47

Hmm...nope, no dynamic sql here. But I did just read some articles about it. I was explicitly DENYing permissions on all SELECT, INSERT, UPDATE, DELETE, and DRI. But this gave me problems. I read that I'd at least have to grant SELeCT permissions to be able to use WHERE clauses (but still did not specify if from a sproc.) I just want to know if for each, SELECT, INSERT, UPDATE, DELETE and DRI, do I have to just revoke for the user? Or can I DENY and still have them work thru the sproc. I tried the DENY scenario for SELECT and it failed. So I assume I can't DENY, but rather REVOKE permissions to view data thru the sproc (btw: user did have GRANT EXEC on the sproc.)

I read that DENY takes precedence, but it did not say thru a sproc (sry I think i said that like 5 times) hehe. That's all I can think of, but if it sounds confusing let me know so I can try to clarify a little more. Thanks.

- RoLY roLLs

tkizer
Almighty SQL Goddess

38200 Posts

Posted - 2004-04-22 : 14:02:35

You do not need SELECT for WHERE clauses in a stored procedure. What articles state that? Why are you denying permissions? Just don't grant them in the first place? To remove a previously granted permission, use REVOKE instead. DENY is causing your problem.

Tara

RoLYroLLs
Constraint Violating Yak Guru

255 Posts

Posted - 2004-04-22 : 15:53:17

quote:
Originally posted by tduggan

You do not need SELECT for WHERE clauses in a stored procedure. What articles state that?

I read it in BOL "DELETE, DELETE (described)" just above the examples: "SELECT permissions are also required if the statement contains a WHERE clause." It is there for UPDATE too, but I assume this is for dynamic sql, no?

quote:
Originally posted by tduggan

Why are you denying permissions? Just don't grant them in the first place? To remove a previously granted permission, use REVOKE instead. DENY is causing your problem.

Well I guess I don't really need to DENY them. But I was only testing diferent scenarios. I also found that if I REVOKE, as well as DENY, permissions on SELECT, I get this: "SELECT permission denied on object 'Maintenance', database 'buysellmotors', owner 'dbo'." This is a table I have that specifies whether the site is under maintenance or not. I call the following sproc:

CREATE procedure rolyrolls.proc_GetMaintenance
	
as
	declare @Maintenance bit
begin
		select
			@Maintenance = m.Maintenance
		from
			Maintenance m
end

return @Maintenance

GO

On the following table:

CREATE TABLE [dbo].[Maintenance] (
	[Maintenance] [bit] NOT NULL 
) ON [PRIMARY]
GO

with the following permission:

GRANT  REFERENCES ,  SELECT ,  UPDATE ,  INSERT ,  DELETE  ON [dbo].[Maintenance]  TO [MYUSERNAME]
GO

I run it fine when logged in as the username I specify as 'MYUSERNAME' but not on the others. When I GRANT SELECT for the other user THEN the user does not get the error.

I know this has no WHERE clause, but I doubt that's the issue I'm trying to work with.

- RoLY roLLs

tkizer
Almighty SQL Goddess

38200 Posts

Posted - 2004-04-22 : 16:02:51

You don't need SELECT permissions on tables regardless if it has a WHERE if the SELECT is done inside a stored procedure. You just need EXEC permissions on the stored procedure. The exception is with dynamic sql.

For your example that you posted, do this:

GRANT EXEC on proc_GetMaintenance TO UserName

instead of what you have for GRANT.

Why is your stored procedure owned by a user other than dbo?

Tara

RoLYroLLs
Constraint Violating Yak Guru

255 Posts

Posted - 2004-04-22 : 16:12:34

quote:
Originally posted by tduggan

Why is your stored procedure owned by a user other than dbo?

Oh, my database resides on a shared sql server. And by changing the owner to the tables to me, rolyrolls, it's a huge change in my sprocs where as i don't mind having the ownership of the sprocs to me.

Oh yeah, I forgot to add the permissions for the sproc:

GRANT  EXECUTE  ON [RoLYroLLs].[proc_GetMaintenance]  TO [OTHERUSER]
GO

The user has had those permissions, and still getting that error.

- RoLY roLLs

tkizer
Almighty SQL Goddess

38200 Posts

Posted - 2004-04-22 : 16:16:08

Could you right click on the user inside the database in EM and go to all tasks, manage permissions. Does it show deny for that table? What does it show for that user?

Tara

RoLYroLLs
Constraint Violating Yak Guru

255 Posts

Posted - 2004-04-22 : 16:28:48

That user has REVOKE on that table

- RoLY roLLs

tkizer
Almighty SQL Goddess

38200 Posts

Posted - 2004-04-22 : 16:35:12

Remove the REVOKE. It should just show a gray box in that GUI window. It should not have a green check on it or a red thing on it, just gray box.

Tara

RoLYroLLs
Constraint Violating Yak Guru

255 Posts

Posted - 2004-04-22 : 16:41:43

REVOKE is the grey box, at least it is to me. DENY = red x, and GRANT = green check, while REVOKE means just the box.

- RoLY roLLs

tkizer
Almighty SQL Goddess

38200 Posts

Posted - 2004-04-22 : 17:02:17

Run this in Query Analyzer as is, do not change anything:

SET NOCOUNT ON

EXEC sp_addlogin @loginame = 'tduggan', @passwd = 'password'
GO

USE pubs
GO

EXEC sp_grantdbaccess 'tduggan', 'tduggan'

CREATE TABLE dbo.Table1
(
Test INT
)
GO

INSERT INTO dbo.Table1 (Test) VALUES(1)
INSERT INTO dbo.Table1 (Test) VALUES(2)
INSERT INTO dbo.Table1 (Test) VALUES(3)
GO

CREATE PROC dbo.usp_SomeProc
AS

SELECT Test
FROM Table1

RETURN

GO

GRANT EXEC ON usp_SomeProc TO tduggan

Now log into your server using tduggan account (sql authentication) with password of password. Now run this:

EXEC pubs.dbo.usp_SomeProc

Do you get this as the result set:

Test
-----------
1
2
3

?

To clean up the objects that I created run this (but not with the tduggan account as it is does not have permissions):

DROP PROC usp_SomeProc
DROP TABLE Table1

EXEC sp_revokedbaccess 'tduggan'
EXEC sp_droplogin 'tduggan'

Tara

Next Page

Subscribe to SQLTeam.com

SQLTeam.com Articles via RSS

SQLTeam.com Weblog via RSS

- Advertisement -

Resources