SQL Injection is not an Indata Validation Problem

Dear Junior

If DDS style use of value objects solves the indata validation problem , and if DDS style indata validation does not solve SQL Injection, then there is only one logical conclusion to draw.

SQL Injection is not an indata validation problem. Yes, that might be contrary to popular belief, but it is obviously so.

If not an indata validation problem, what kind of beast is it? And what can we do about it?

Yours

Dan

Indata Validation is Not Enough for SQL Injection

Dear Junior

When we do indata validation through value objects we get an application tier that is water-proof. The model describes exactly those data that we think are meaningful and can handle (“username, identifier with which the user present herself to the system; regexp [a-z]+“. Each piece of indata is validated as part of the value object constructor (public Username(String s)). For indata to pass through to application, it cannot avoid validation, as the application methods require the value object type (void authenticate(Username uname…)). What more can you ask for?

- Well, you see, we do not want usernames like “danbj”, we would prefer our real names, like “Dan Bergh Johnnsson”.

No problem, we expand the regexp with uppercase letters and space, getting a regexp like [a-zA-Z\ ]+.

- Nice, but our respected colleague Fredrik Jägare-Lilja needs a username as well.

Fair enough – we stuff Scandinavian letters and hyphen into the regexp as well, giving [a-zåäöA-ZÅÄÖ\ \-]+.

- Now there is only one person left: our highly respected Irish colleague Oliver O’Hehir.

Well, well, we are almost finished then, we only need to put the apostrophe into the regexp ending up with something along [a-zåäöA-ZÅÄÖ\ \-\’]+.

Wait, wait, wait!!! Who the h*** just logged in with username “’ OR ‘a’ is not null --“?

Sure, we might have tightened up the regexp to block out that specific attack string and any other malicious use of the format we can think of. But, it is always those we did not think of that causes the trouble.

Well, as always we can think that “SQL Injection is solved by prepared statements”, but remember that Injection Flaw is much larger than SQL Injection. The same vulnerabilities might be there when doing LDAP access, using parameters to construct file names (e g Directory Traversal), or if you have some Domain Specific Language (DSL) which you interpret. In any of these cases there might be a string that might well be fully legally formatted, but attacks the structure of how the underlying resources are used.

Over to a completely different domain: FM broadcast and music radio. In the FM radio broadcast system you must be able to shut down the transmitters from a remote site. Unfortunately, there is only one way to communicate with the transmitters – via radio. The problem was solved by defining a specific sequence of audio blips (very precise on frequencies, duration, and interval) and denoting that sequence the meaning “shut down the transmitter”.

The pioneering Swedish rap group JustD put that exact sequence as the final beat on one of their songs, without telling anyone. They must have laughed all the way home from the studio. That song has been played on Swedish radio exactly once.

The JustD track hack is a wonderful example of exploiting an Injection Flaw, there is no way to escape it “in band”. I have the same gut feeling about indata validation and SQL Injection.

No matter how we structure the indata model, there might always be some data that actually is valid indata, but causes the system to crash.

So, indata modelling and validation in all its glory: However necessary it is for upholding security, it is not sufficient.

Yours

Dan

Util Methods does not Work

Dear Junior

When writing the validation logic for the username

public boolean isValid() {

  return username.matches("[a-z]+");

}

I suddenly heard a distant screaming: “Why did you not use the util method for validation?” Ehhrr … sorry … which method? Ohhh, over there … in the se.xyz.services.util.stringutils package there is a util class StringValidationUtil with a validation method.

public class StringValidationUtil {

  static public boolean logincheck(String username) {

    if (!(username.length() > 0)) return false;

    for(int i=0; i<username.length();i++)

      if(!Character.isLowerCase(username.charAt(i)))

        return false;

    return true;

  }

}

I am sorry, I guess I just didn’t find it.

It is strange that I did not find it, because it is actually called as part of account creation when registering a new user. Did I not look for it properly?

Well, it is also strange that I did not find the method in se.xyz.utils.security.AccountTransformUtils, because there you can find

static public boolean okNewUsername(String username) {

  boolean result = true;

  if (username.length() == 0) result = false;

  for(int i=0; i<username.length();i++)

    result = result && Character.isLowerCase(username.charAt(i));

  return result;

}

That method is by the way also called, as part of the check when someone wants to change username. Did I not look properly for that either?

And of course there are some more methods in se.xyz.accmgm.AccountUtil and in the ever-present se.xyz.util.Util that all basically do the same thing - check that an account name has the proper form.

My real-life record was a util class that contained five different implementations of checking that a string was a date on the format “YYYYmmDD” – and between those implementations, there where subtle differences when handling some strange cases. By the way, there where also three more different implementations in another slightly differently named util class as well.

So, how come this multitude of util methods? They are simply not found! And the programmer in need for validating that there are only lowercase letters in the string at hand will probably look for the needed method for ten to thirty seconds, where after she will implement it herself – after all it is not that difficult. Then, to make “my nice method helpful for everybody else” it is moved to some util class.

As a side-note, you can note that most util methods are ‘static’. To me ‘static’ in an oo-program means “homeless”. Those methods could reside equally well in any other class. And residing in some obscure hide-away package does not make them easy to find.

For a method to be used, it must be in the middle of the road where the programmer is going. That is what object-orientation is good at, the methods are hung up on the data you have in your hands, so the methods are easy to find.

But unfortunately, static util methods do not work that way. They simply do not work.

Yours

Dan

ps A better place to put validation is inside the corresponding value object

Ensuring Indata Validation

Dear Junior

Creating a username class and a validation method has taken us a fair amount towards solving SQL Injection by focusing on a domain model API that is both easy to use correctly and hard to use incorrectly. I would say that we have this far achieved to make the API easy to use.

  Integer authenticate(Username username, String passwordMD5)  
 public class Username {
  // final making it immutable
  public final String username; 
  public Username(String username) { this.username = username; }
  public boolean isValid() { return username.matches("[a-z]+"); }
}
What remains is to ensure that indata validation actually is done. I can see two choices: either putting validation inside the authentication service, or to enforce validation before the call to the authentication service.

Let us first look at putting validation inside authentication.

  /** Authenticates a user with a given password.
   * @throws IllegalArgumentException if username invalid
   */
  Integer authenticateWithUsernameValidation(Username username, String passwordMD5)
          throws IllegalArgumentException, SQLException {
    if(!username.isValid())
      throw new IllegalArgumentException(
              "Cannot authenticate with invalid username: " + username);
    ...
  }

This definitely hardens the interface – now there is no possibility to not validate upon authentication. However, the same trick has to be used in every service method around, including the “create new account”, the “change account username” and all those that are to come in the future. Risks are high that the small isValid-call will be missed somewhere – and one hole is all an attacker needs.

Another drawback is the rather awkward “throws IllegalArgumentException” which feels like a very late validation – should not such validation be made much earlier, preferably up in the presentation and client tiers?

An alternative is to not allow invalid usernames to be constructed at all:

  @Test(expected = IllegalArgumentException.class)
  public void shouldNotCreateUsernameFromInjectionAttackString() {
    new Username("' OR 1=1 --");
  }

This request the constructor to do the validation on the inside, responding with an exception if given an invalid username candidate.

  public Username(String username)
    throws IllegalArgumentException {
    this.username = username;
    if(!isValid())
      throw new IllegalArgumentException();
  }

Now we also need some way to validate from the outside without taking the pain of provoking and handling an exception, so finally there will be a static method after all:

  public static boolean isValid(String username) {
    return username.matches("[a-z]+");
  }

Of course the old methods and constructor should be refactored to uphold the don’t-repeat-yourself (DRY) principle. Interesting enough this will lead the isValid() method to consistently return true – so I guess we can delete it from the class and inline it wherever it was used. That is, unless we for some bizarre reason want to have a method that explicitly tells the rest of the world that “this object is always valid”.

I definitely prefer this latter “strictly-validated-value-object” style before the "validating-service-methods" style. It creates an API that besides being easy to use correctly, also is hard to use incorrectly. It “guides” the client side programmer without being intrusive or obstructive about it.

In some sense, it "enforce" a behaviour upon the client side programmer. However, that does not trouble me. If someone just is nice to me, and don’t cause me trouble, I see no obstacle in letting her have her way.

Yours

Dan

Canonical Data Format for Domain Model Data

Dear Junior

When accepting data from a user, it can come in variations of forms, and the same data can come in different forms. E g, a phone number can be sent as "0709-158843", "0709 158 843", or "+46(0)709158843", all denoting the same thing - the phone number to my mobile phone. This can really be a hassle - say that one user stores the phone number as "0709 158 843" an another searches for people with phone number "0709-158843"; then there is a risk that there will be no match, even though it is actually the same phone number.

This situation can get even subtler: non-7-bit-ascii characters like the Swedish letters å, ä, and ö can be represented as bytes in several different ways. Thus, there is always a risk that the search-form does not use the same encoding as the storage in the database. The result might again be "no match" even though the two users (one entering a name like "Öberg", another searching for it) punched exactly the same keys in the same sequence, perhaps even using the same physical keyboard.

To get around this, I usually decide a canonical data format to be used in the model, which naturally also will be in the code that implementing the model. And now I do not mean technical issues like using Unicode character encoding, those are up to the programmers to decide. I mean what a phone number looks like "in its own nature", so to say "beneath different representations".

What a phone number should look like is a domain modelling issue, and should be decided together with the domain experts (user representative, product owner, whoever ...). It is often pretty efficient to bring up the problem explicitly: "We need to set a standard for what phone numbers should look like, otherwise the application will look like a mess, searches will fail, and integration with other systems will be a nightmare. I have a few suggestions for possible alternatives: is there anyone you like better then the other, or are there another format you would suggest?" Probably you will come out with something like "0709-158843", which can be abstracted to the regexp "0ddd-dddddd[d]*" or similar. This is your canonical form, and will in Domain Driven Design terminology be part of your domain model.

When choosing your canonical form and how to represent and store it, you must obviously take a look at the system as a whole. Probably your choice will be guided both by functionality you want to provide as well as system qualities (NFRs) such as capacity, response time, and security. E g if you are managing a forum site with discussions that are mainly held in only in English, you might restrict comments to only contain A-Z, a-z, some white-spaces and some punctuations. If you have several languages, but mostly west-European, you might not be able to restrict the ranges, but can store it using UTF-8 (for storage capacity), if there are a lot of other you might use UTF-16. Finally, if the content is sole for publishing on web and it can contain characters like '<', you might HTML-encode it for security reasons.

After deciding canonical form and its representation, it will be the responsibility for any indata handler to validate incoming data and convert it to the canonical form. The logic for doing this can preferably be put into a value object class (PhoneNumber) so that it is easily found and used. Then the rest of the application can safely use (as field declarations, variables, arguments and return values) this type, making the rest of the code more precise and expressive.

Also, by default all data presentations (i e output) will be on this format as well. If there are presentations that need another form, the responsibility falls on them to convert to the format needed. E g some listing might want all phone numbers to be structured as "0709 [tab] 15 88 43", then it is up to that listing to convert the phone numbers to that format. In the same way, if some presentation needs a specific encoding (UTF-16, Base-64 or HTML-encoded), it is up to that presentation tier to do the conversion.

In this way, the life within the model becomes simple, searching and matching can be done with out trouble. At the same time, we can facilitate all the input and output formats we need by pushing coding, and conversions towards the system boarder.

Yours

Dan