SQL injection in unexpected places
Every so often, I write about some real-world problems in this blog, rather than just getting excited about generalities. This is one of those times.
1. In which I am an idiot who thinks he is clever
I had a list of users the other day, exported from a partner with whom we do SSO, and which somehow had some duplicate entries in.
These were not duplicate in the sense of âexactly the same data in every fieldâ, but differed by email address, and sometimes last name. Those of you who manage identity databases will know exactly what Iâm dealing with here â people change their last name, through marriage, divorce, adoption, gender reassignment, whim or other reason, and instead of editing the existing entry, a new entry is somehow populated to the list of identities.
What hadnât changed was that each of these individuals still held their old email address in Active Directory, so all I had to do was look up each email address, relate it to a particular user, and then pull out the canonical email address for that user. [In this case, thatâs the first email address returned from AD]
A quick search on the interwebs gave me this as a suggested VBA function to do just that:
1: Function GetEmail(email as String) as String
2: ' Given one of this users' email addresses, find the canonical one.
3:
4: ' Find our default domain base to search from
5: Set objRootDSE = GetObject("LDAP://RootDSE")
6: strBase = "'LDAP://" & objRootDSE.Get("defaultNamingContext") & "'"
7:
8: ' Open a connection to AD
9: Set ADOConnection = CreateObject("ADODB.Connection")
10: ADOConnection.Provider = "ADsDSOObject"
11: ADOConnection.Open "Active Directory Provider"
12:
13: ' Create a command
14: Set ADCommand = CreateObject("ADODB.Command")
15: ADCommand.ActiveConnection = ADOConnection
16:
17: 'Find user based on their email address
18: ADCommand.CommandText = _
19: "SELECT distinguishedName,userPrincipalName,mail FROM " & _
20: strBase & " WHERE objectCategory='user' and mail='" & email & "'"
21:
22: ' Execute this command
23: Set ADRecordSet = ADCommand.Execute
24:
25: ' Extract the canonical email address for this user.
26: GetEmail = ADRecordSet.Fields("Mail")
27:
28: ' Return.
29: End Function
That did the trick, and I stopped thinking about it. Printed out the source just to demonstrate to a couple of people that this is not rocket surgery.
2. In which I realise I am idiot
Yesterday the printout caught my eye. Hereâs the particular line that made me stop:
18: ADCommand.CommandText = _
19: "SELECT distinguishedName,userPrincipalName,mail FROM " & _
20: strBase & " WHERE objectCategory='user' AND mail='" & email & "'"
That looks like a SQL query, doesnât it?
Probably because it is.
Itâs one of two formats that can be used to query Active Directory, the other being the less-readable LDAP syntax.
Both formats have the same problem â when you build the query using string concatenation like this, itâs possible for the input to give you an injection by escaping from the data and into the code.
I checked this out â when I called this function as follows, I got the first email address in the list as a response:
1: Debug.Print GetEmail("x' OR mail='*")
You can see my previous SQL injection articles to come up with ideas of other things I can do now that Iâve got the ability to inject.
3. In which I try to be clever again
Normally, Iâd suggest developers use Parameterised Queries to solve this problem â and thatâs always the best idea, because it not only improves security, but it actually makes the query faster on subsequent runs, because itâs already optimised. Hereâs how that ought to look:
1: ADCommand.CommandText = _
2: "SELECT distinguishedName,userPrincipalName,mail FROM " & _
3: strBase & "WHERE objectCategory='user' AND mail=?"
4:
5: 'Create and bind parameter
6: Set ADParam = ADCommand.CreateParameter("", adVarChar, adParamInput, 40, email)
7: ADCommand.Parameters.Append ADParam
That way, the question mark â?â gets replaced with â’youremail@example.com’â (including the single quote marks) and my injection attempt gets quoted in magical ways (usually, doubling single-quotes, but the parameter insertion is capable of knowing in what way itâs being inserted, and how exactly to quote the data).
4. In which I realise other people are idiot
Thatâs the rather meaningful message:
Run-time error â-2147467262 (80004002)â:
No such interface supported
It doesnât actually tell me which interface is supported, so of course I spend a half hour trying to figure out what changed that might have gone wrong â whether Iâm using a question mark where perhaps I might need a named variable, possibly preceded by an â@â sign, but no, thatâs SQL stored procedures, which are almost never the SQL injection solution they claim to be, largely because the same idiot who uses concatenation in his web service also does the same stupid trick in his SQL stored procedures, but Iâm rambling now and getting far away from the point if I ever had one, soâŠ
The interface that isnât supported is the ability to set parameters.
The single best solution to SQL injection just plain isnât provided in the ADODB library and/or the ADsDSOObject provider.
Why on earth would you miss that out, Microsoft?
5. I get clever
So, the smart answer here is input validation where possible, and if you absolutely have to accept any and all input, you must quote the strings that youâre passing in.
In my case, because Iâm dealing with email addresses, I think I can reasonably restrict my input to alphanumerics, the â@â sign, full stops, hyphens and underscores.
Input validation depends greatly on the type of your input. If itâs a string, that will need to be provided in your SQL request surrounded with single quotes â that means that any single quote in the string will need to be encoded safely. Usually that means doubling the quote mark, although you might choose to replace them with double quotes or back ticks.
If your input is a number, you can be more restrictive in your input validation â only those characters that are actually parts of a number. Thatâs not necessarily as easy as it sounds â the letter âeâ is often part of numbers, for instance, and you have to decide whether youâre going to accept bases other than 10. But from the perspective of securing against SQL injection, again thatâs not too difficult to enforce.
Finally, of course, you have to decide what to do when bad input comes in â an error response, a static value, throw an exception, ignore the input and refuse to respond, etc. If you choose to signal an error back to the user, be careful not to provide information an attacker could find useful.
Whatâs useful to an attacker?
Sometimes the mere presence of an error is useful.
Certainly if you feed back to the attacker the full detail of the SQL query that went wrong â and people do sometimes do this! â you give the attacker far too much information.
Even feeding back the incorrect input can be a bad thing in many cases. In the Excel case Iâm running into, thatâs probably not easily exploitable, but you probably should be cautious anyway â if itâs an attacker causing an error, they may want you to echo back their input to exploit something else.
Call to Microsoft
Seriously, Microsoft, this is an unforgiveable lapse â not only is there no ability to provide the single best protection, because you didnât implement the parameter interface, but also your own samples provide examples of code that is vulnerable to SQL injections. [Here and here â the other examples I was able to find use hard-coded search filters.]
Microsoft, update your samples to demonstrate how to securely query AD through the ADODB library, and consider whether itâs possible to extend the provider with the parameter interface so that we can use the gold-standard protection.
Call to developers
Parse your parameters â make sure they conform to expected values. Complain to the user when they donât. Donât use lack of samples as a reason not to deliver secure components.
Finally â how I did it right
And, because I know a few of you will hope to copy directly from my code, hereâs how I wound up doing this exact function.
Please, by all means review it for mistakes â I donât guarantee that this is correct, just that itâs better than I found originally. For instance, one thing it doesnât check for is if the user actually has a value set for the âmailâ field in Active Directory â I can tell you for certain, itâll give a null reference error if you have one of these users come back from your search.
1: Function GetEmail(email As String) As String
2: ' Given one of this users' email addresses, find the canonical one.
3:
4: ' Pre-execution input validation - email must contain only recognised characters.
5: If email Like "*[!a-zA-Z0-9_@.]*" Then
6: GetEmail = "Illegal characters"
7: Exit Function
8: End If
9:
10:
11: ' Find our default domain base to search from
12: Set objRootDSE = GetObject("LDAP://RootDSE")
13: strBase = "'LDAP://" & objRootDSE.Get("defaultNamingContext") & "'"
14:
15: ' Open a connection to AD
16: Set ADOConnection = CreateObject("ADODB.Connection")
17: ADOConnection.Provider = "ADsDSOObject"
18: ADOConnection.Open "Active Directory Provider"
19:
20: ' Create a command
21: Set ADCommand = CreateObject("ADODB.Command")
22: ADCommand.ActiveConnection = ADOConnection
23:
24: 'Find user based on their email address
25: ADCommand.CommandText = _
26: "SELECT distinguishedName,userPrincipalName,mail FROM " & _
27: strBase & " WHERE objectCategory='user' AND mail='" & email & "'"
28:
29: ' Execute this command
30: Set ADrecordset = ADCommand.Execute
31:
32: ' Post execution validation - we should have exactly one answer.
33: If ADrecordset Is Nothing Or (ADrecordset.EOF And ADrecordset.BOF) Then
34: GetEmail = "Not found"
35: Exit Function
36: End If
37: If ADrecordset.RecordCount > 1 Then
38: GetEmail = "Many matches"
39: Exit Function
40: End If
41:
42: ' Extract the canonical email address for this user.
43: GetEmail = ADrecordset.Fields("Mail")
44:
45: ' Return.
46: End Function
As always, let me know if you find this at all useful.
Leave a Reply