April 12, 2013 at 1:03 pm
Please post the stored procedure and how you are invoking it as well.
April 12, 2013 at 1:07 pm
I tried to post the entire stored proc, but something in it was causing a problem with the forum. I suspect it's either the table creation or dropping or the invoking of the execute itself. If someone knows how I can get around this, I'll happily post the whole thing. As for how I'm invoking it, if I do
exec getAffectedEmployees "'099999-00005','099999-00004'"
from withing SSMS, I get no results. However, if I copy the actual code from the sp to a query window in SSMS, I get a result set.
April 12, 2013 at 1:10 pm
It's late on a Friday. Someone on another forum just pointed out to me that I wasn't doing a SELECT on my temp table after I built it. I'm going to go slam my head into a wall now. Happy weekend everyone!
April 12, 2013 at 1:18 pm
Melanie Peterson (4/12/2013)
It's late on a Friday. Someone on another forum just pointed out to me that I wasn't doing a SELECT on my temp table after I built it. I'm going to go slam my head into a wall now. Happy weekend everyone!
BE VERY CAREFUL. From what you posted and described you are receiving a parameter and putting that directly into a dynamic string and executing it. That is textbook sql injection vulnerability. If you want to pass in a delimited string like that it is FAR better and safer to parse that string instead of just sticking it inside a string and executing. I am guessing the reason you are using dynamic sql in the first place is because you want to pass a comma delimited list to your sproc and this was the only you could figure out how to make that work. It sounds like your proc needs a major overhaul.
_______________________________________________________________
Need help? Help us help you.
Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.
Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.
Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/
April 12, 2013 at 1:19 pm
Melanie Peterson (4/12/2013)
I tried to post the entire stored proc, but something in it was causing a problem with the forum. I suspect it's either the table creation or dropping or the invoking of the execute itself. If someone knows how I can get around this, I'll happily post the whole thing. As for how I'm invoking it, if I doexec getAffectedEmployees "'099999-00005','099999-00004'"
from withing SSMS, I get no results. However, if I copy the actual code from the sp to a query window in SSMS, I get a result set.
That's what I was thinking may be the problem. Glad you got it sorted.
April 12, 2013 at 1:25 pm
SSCrazy Eights - thanks for the warning. The input to this stored procedure, however, is done entirely through choices made in lists populated from the database. The user never has the opportunity to input anything. Furthermore, it's not an app on an outward facing website, just something internal to our rather small company and well within a firewall, DMZ, etc., so I'm not really concerned about SQL injection here. It's always a good warning though, and after what I've just said you still think it should be handled differently, feel free to say so! 🙂
April 12, 2013 at 1:32 pm
Can you try uploading your procedure in a text file (*.txt)? I had issues like this at a previous employer and that was the only way to provide code sometimes.
April 12, 2013 at 1:39 pm
Melanie Peterson (4/12/2013)
SSCrazy Eights - thanks for the warning. The input to this stored procedure, however, is done entirely through choices made in lists populated from the database. The user never has the opportunity to input anything. Furthermore, it's not an app on an outward facing website, just something internal to our rather small company and well within a firewall, DMZ, etc., so I'm not really concerned about SQL injection here. It's always a good warning though, and after what I've just said you still think it should be handled differently, feel free to say so! 🙂
You might be reasonably safe then at least until this has to be opened up to people at home. 🙂
The change is fairly easy actually. I assume that your variable @CLM is what contains your delimited list? This query will return the same thing but it is not vulnerable to injection and it doesn't need dynamic sql either. It does however use the DelimitedSplit8K function. You can find out more about that function and the code to generate it by following the link in my signature about splitting strings.
SELECT t.[EmpLogin],t.Matter, e.AttorneyAndTitle, SUM([Workhours]) AS TotalHours,
c.ClientName + '/' + em.MatterDesc --not quite sure what this was doing
FROM TimeCard t
INNER JOIN Emps e ON e.EmpLogin = t.EmpLogin
INNER JOIN EliteMatter em ON t.Matter = em.MatterName
INNER JOIN EliteClient c ON c.ClientNum = em.ClientNum
INNER JOIN dbo.DelimitedSplit8K(@CLM, ',') x on x.Item = t.Matter
GROUP BY t.EmpLogin, c.ClientName, em.MatterDesc, t.Matter, e.AttorneyAndTitle
ORDER BY TotalHours
_______________________________________________________________
Need help? Help us help you.
Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.
Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.
Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/
April 12, 2013 at 1:54 pm
SSCrazy Eights - thanks very much!
Lynn - I'd love to upload the file, but I can't figure out how to do that either. I think it must be Friday. :blush:
April 12, 2013 at 4:13 pm
Look down to attachments while writing a post.
April 15, 2013 at 8:31 am
OK, it's Monday, brain is (more or less) refreshed and I'm posting the text of the sp, including the T-SQL that seemed to be causing the code to not post to this forum. I'm also including the SELECT statement that I needed to get the thing to work properly, and would have eliminated the need for this post in the first place. :blush:
April 15, 2013 at 8:47 am
Melanie Peterson (4/15/2013)
OK, it's Monday, brain is (more or less) refreshed and I'm posting the text of the sp, including the T-SQL that seemed to be causing the code to not post to this forum. I'm also including the SELECT statement that I needed to get the thing to work properly, and would have eliminated the need for this post in the first place. :blush:
I still say you should look into the DelimitedSplit8K function. It eliminates the need for dynamic sql and you have zero risk of sql injection. It will run just as fast but it is way simpler to read.
Having looked at your code you don't need the temp table either. Your entire proc becomes this simple.
CREATE PROCEDURE [dbo].[GetLitHoldAffectedEmployees]
@CLM as NVARCHAR(max)
AS
SET NOCOUNT ON
;WITH Emps (EmpLogin, AttorneyAndTitle) AS (
SELECT EmpLogin,
CASE WHEN EmpTermDate IS NULL OR EmpTermDate = '' THEN LastName + ', ' + FirstName + ' [' + Title + '] '
ELSE LastName + ', ' + FirstName + ' [' + Title +'] (inactive) ' END as FullName
FROM [LitigationHold].[dbo].[Employees]
where JobCode IN ('100','300','400','500','700','800','1801'))
SELECT t.[EmpLogin],t.Matter, e.AttorneyAndTitle, SUM([Workhours]) AS TotalHours,
c.ClientName + '/' + em.MatterDesc
FROM TimeCard t
INNER JOIN Emps e ON e.EmpLogin = t.EmpLogin
INNER JOIN EliteMatter em ON t.Matter = em.MatterName
INNER JOIN EliteClient c ON c.ClientNum = em.ClientNum
INNER JOIN dbo.DelimitedSplit8K(@CLM, ',') s on s.Item = t.Matter
GROUP BY t.EmpLogin, c.ClientName, em.MatterDesc, t.Matter, e.AttorneyAndTitle
ORDER BY TotalHours
Test is out but I think that will produce the exact same results.
_______________________________________________________________
Need help? Help us help you.
Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.
Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.
Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/
Viewing 12 posts - 1 through 13 (of 13 total)
You must be logged in to reply to this topic. Login to reply