3

I recently answered a question with the following code sample:

Create Table #Testing
(
  emaildomain varchar(100) -- Still bigger than functionally needed but better than MAX
);
INSERT INTO #Testing VALUES (REVERSE('@myfreepaysite.com')); -- Changed


Create Table #DataToCheck
(
  fullemail varchar(200) -- Still bigger than functionally needed but better than MAX
);
Insert Into #DataToCheck VALUES(REVERSE('[email protected]')); --Changed

Select Top 1 
  REVERSE(fullemail)
  , REVERSE(emaildomain)
FROM #DataToCheck
  INNER JOIN #Testing ON fullemail LIKE emaildomain + '%';

As I was writing that last line I wondered if concating EmailDomain column with the wildcard symbol was an opening for SQL injection. Is this something I should watch out for this type of code or will SQL Server evaluate this as a single unified parameter so I don't need to worry about using this approach?

3
  • A couple of comments, I know this isn't part of your question but (a) eckes is right that specifically here hope.com will match nohope.com, and (b) doing the reverse and trailing wildcard doesn't help with varchar(max) because you can't index that anyway (trailing wildcard helps you potentially get a narrow seek, on an index). That doesn't mean the approach is wrong, just that the data type choice is bad. There are RFCs / standards that dictate the max length of a domain name and the local part of an e-mail address, and it's a lot less than 2GB. :-) Apr 12, 2018 at 20:47
  • @AaronBertrand I just updated the answer that I pulled this source from to include the '@' symbol which should fix the hope.com/nohope.com bug. With point b) are you saying this approach is useless, or only marginally helpful? I agree that functionally it is absurd to have a VARCHAR(MAX) for an email. :)
    – Erik
    Apr 12, 2018 at 20:52
  • Just an observation that reversing a varchar(max) so that the wildcard is trailing instead of leading doesn't change anything, because a varchar(max) can't be indexed so a seek won't happen in any case. So if the OP is reversing all of those values solely to make the query more efficient, yes, it is useless. Apr 17, 2018 at 12:21

1 Answer 1

5

No. That value will always be interpreted as a string and there's no way to put anything into the emaildomain column that could make the query execute unexpected code. It's either going to find matches or it's not.

Of course, this is relevant to the specific code sample you posted, and only that. You should always be concerned about any data you accept from a user... since you might not know where else it might be used now or in the future.

4
  • However the query makes not much sense, it might be suspectible to extension/missing delimiter attacks and has not very great performance ‚example.co‘ would match ‚example.com‘
    – eckes
    Apr 12, 2018 at 19:51
  • Ok in that case Hope.com and Nohope.com
    – eckes
    Apr 12, 2018 at 20:09
  • @Erik No the problem comes when you do (in your app or your stored procedure) SET @sql = N'SELECT ...' + @userinput; Apr 12, 2018 at 20:37
  • @AaronBertrand perfect. Thank you. I knew your example in the comment was vulnerable.
    – Erik
    Apr 12, 2018 at 20:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.