In a previous post, I talked about making a test list. People have emailed me saying they don't see the path from the test list to an implementation. Here's the next step.

---

In a previous post , I talked about a test list for finding a very basic kind of unmanaged code security vulnerability. "How do we take that and drive an implementation?" is the question I got in several emails. For those attending the BlackHat class or Defcon talks , getting this to compile and pass using SharpDevelop or mono would be good practice before attending those events.

Well, let's start at the beginning with the first test from the test list:

void *p = malloc(16);
p[16] = 0; // BUG

First, I want to decide on a good metaphor for the region of memory encapsulated by the pointer. Since this is in the context of buffer overflows, let's call it a Buffer. A Buffer gets initialized with a set size (for now). If a byte outside of the size of the Buffer is accessed, let's throw an exception (for now). (Much later, you could translate source or binary code into these kinds of high-level metaphors if you were building a code analysis tools.) The first test (in the style of NUnit):

[Test]
[ExpectedException(typeof(IndexOutOfRangeException))]
public void BufferAssignmentOneBeyondEnd()
{
  Buffer buffer = new Buffer(16);
  buffer[16] = 0;
}

Okay, that test doesn't compile. Nevermind the missing namespace/class enclosure for the test method and the missing using statements for System and NUnit.Framework. I mean there is no Buffer class at all. Our first step is to do just enough to make the test compile.

public class Buffer
{
  public Buffer(UInt32 _size)
  {
  }

  public byte this[UInt32 index]
  {
    set { }
  }
}

The test compiles

matt@pretention:~/src$ mcs -debug -r:nunit.framework -r:System -t:library buffer.cs

and runs... but fails.

matt@pretention:~/src$ nunit-console buffer.dll
NUnit version 2.2.0
Copyright (C) 2002-2003 James W. Newkirk, Michael C. Two, Alexei A. Vorontsov, Charlie Poole.
Copyright (C) 2000-2003 Philip Craig.
All Rights Reserved.

OS Version: Unix 2.6.15.25    Mono Version: 1.1.4322.2032

.F

Tests run: 1, Failures: 1, Not run: 0, Time: 0.103694 seconds

Failure. Good. That's what we expected. You always want to try and see your test fail -- that tests your test. That is, it is possible to fat finger the keyboard or not be thinking and write a test that always passes. Which is not, of course, what most people intend. Tests that pass when we don't except them to are just as bad as test that fail when we don't expect them to. You are no longer one with your program, and if you're pairing then you're both not operating at maximum capacity. If that happens, take a break, nap, walk, play video games, watch a movie, pleasure yourself, read a book of fiction, or something.

Let's make the test pass by adding a throw to our indexed property.

public class Buffer
{
  public Buffer(UInt32 _size)
  {
  }

  public byte this[UInt32 index]
  {
    set { throw new IndexOutOfRangeException(); }
  }
}

Yup, the test now passes:

matt@pretention:~/src$ nunit-console buffer.dll
NUnit version 2.2.0
Copyright (C) 2002-2003 James W. Newkirk, Michael C. Two, Alexei A. Vorontsov, Charlie Poole.
Copyright (C) 2000-2003 Philip Craig.
All Rights Reserved.

OS Version: Unix 2.6.15.25    Mono Version: 1.1.4322.2032

.

Tests run: 1, Failures: 0, Not run: 0, Time: 0.103872 seconds

"But wait!", you say, "this class doesn't do what it claims!" You're right -- it only does what the existing test documents it does. I personally prefer to start with exceptional cases because it is usually easy to make the test pass and that allows us to focus on the structure of the API and the error handling. I hear you, though, so let's generalize a bit with a non-exceptional case.

[Test]
public void BufferAssignmentAtBeginning()
{
  Buffer buffer = new Buffer(16);
  buffer[0] = 0;
  Assert.AreEqual(0, buffer[0]);
}

That fails because our index property's setter always throws IndexOutOfRangeException. That's not appropriate here, so let's fix it and still allow for the other test to pass as well.

public class Buffer
{
  public Buffer(UInt32 _size)
  {
  }

  public byte this[UInt32 index]
  {
    set
    {
      if (0 != index)
        throw new IndexOutOfRangeException();
    }

    get
    {
      return 1;
    }
  }
}

At first, I forgot to add the get{} which is used in the Assert.AreEqual() in the test. For brevity, I just told you about it rather than drag on with detailed illustrations of every one of my minor screw ups. Anyways, the code compiles and the test fails. It failed because the get{} always returns 1 and we are expecting 0. Change the 1 to a 0, and the test passes.

We're still cheating our asses off here, and this can seem very tedious. Once you get into the swing of TDD, maybe you (and your pair) will feel more comfortable moving more quickly. Just realise that it is a very slippery slope when you start talking yourself out of things to test. I prefer to just make it a habit and never question it unless the circumstances are super-duper-crazy dire. Like the building, my clothes, or my pair being on fire.

Okay, one more test to make this to be more real:

[Test]
public void BufferAssignmentAtEnd()
{
  Buffer buffer = new Buffer(16);
  buffer[15] = 1;
  Assert.AreEqual(1, buffer[15]);
}

To speed things up, I am assigning 1 and checking the value exists rather than using 0 as in previous tests. 1 will make us generalize a bit more this time around, and I need to stop writing this post and get on with other things on my TODO list. The test compiles okay -- our interfaces are already in place from the previous tests. This is a hint of the real joy and benefit of TDD -- it allows you to focus on clean interfaces rather than implementation details, almost like modelling in UML.

This test does fail, of course:

matt@pretention:~/src$ mono --debug ~/lib/mono/1.0/nunit-console.exe buffer.dll
NUnit version 2.2.0
Copyright (C) 2002-2003 James W. Newkirk, Michael C. Two, Alexei A. Vorontsov, Charlie Poole.
Copyright (C) 2000-2003 Philip Craig.
All Rights Reserved.

OS Version: Unix 2.6.15.25    Mono Version: 1.1.4322.2032

...F

Tests run: 3, Failures: 1, Not run: 0, Time: 0.145645 seconds

Tests run: 3, Failures: 1, Not run: 0, Time: 0.145645 seconds

Failures:
1) Foo.BufferTests.BufferAssignmentAtEnd : System.IndexOutOfRangeException : Array index is out of range.
  at Foo.Buffer.set_Item (UInt32 index, Byte value) [0x0000c] in /home/matt/src/buffer.cs:18
  at Foo.BufferTests.BufferAssignmentAtEnd () [0x00008] in /home/matt/src/buffer.cs:50
  at <0x00000> <unknown method>
  at (wrapper managed-to-native) System.Reflection.MonoMethod:InternalInvoke (object,object[])
  at System.Reflection.MonoMethod.Invoke (System.Object obj, BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00040] in /home/matt/src/mcs/class/corlib/System.Reflection/MonoMethod.cs:130

Yes, mono's nunit-console output is that ugly . Note how I have to reach into the lib/mono/1.0 directory so I can run with --debug to get line numbers. Blechity-fuqn-blech. Next blog post will be in SharpDevelop, I promise. I didn't need the line numbers, technically, but I don't recommend getting in the habit of not getting that information -- you'll waste a lot of time otherwise. (Especially with legacy code.)

The test failure points us to IndexOutOfRangeException again. Let's fix that first and keep our other tests passing.

public class Buffer
{
  UInt32 size;

  public Buffer(UInt32 _size)
  {
    size = _size;
  }

  public byte this[UInt32 index]
  {
    set
    {
      if (index >= size)
        throw new IndexOutOfRangeException();
    }

    get
    {
      return 0;
    }
  }
}

To do this properly, we finally had to store the size. Note that we don't have to check to see if our index is greater than zero because it's a UInt32, which won't be less than zero by definition. If it ever overflows, we'll know if we compiled with -checked+ . Anyways, this gets rid of the exception and doesn't upset the other passing tests. The last test is now failing because it is assigning a 1 and getting back a 0 -- the only thing our indexed property returns.

It's interesting to note that we could cheat again here. That is, we could only store the last byte assigned and always return that. The tests would all pass, but only by accident. In fact, give that a try -- I'll wait. ...... It passes -- don't you feel great! Wait, you don't? It's probably because you just engaged in something the Pragmatic Programmers call "programming by coincidence" (I highly recommend their book, BTW). We don't intend for that test to pass -- it doesn't express our intentions with regard to this class. We could be happy with our undeserved passing tests -- but then we would have to commit ritual suicide -- leaving a problem like this for later will just crush our buzz. We could just cheat and write another test, but let's just do it for real this time. Again, this is driven by me needing to be getting elsewhere. And my head is on fire.

Making the test pass for reals:

public class Buffer
{
  byte[] storage;

  public Buffer(UInt32 _size)
  {
    storage = new byte[_size];
  }

  public byte this[UInt32 index]
  {
    set
    {
      if (index <= storage.Length)
        throw new IndexOutOfRangeException();

      storage[index] = value;
    }

    get
    {
      return storage[index];
    }
  }
}

b00m. All the tests now pass. Note that I didn't keep the UInt32 size around. That's because it would have duplicated the array Length property -- remember, Don't Repeat Yourself. You might notice that assignments to the storage are guarded against being past the array bounds, but reads aren't. As I stated in the previous blog post, we only really care about OOB pointer writes right now. Besides, it'll be really easy to add a test to document that intent and make it pass.

Some of you might be saying "Wait a minute! Why not just use a regular array and its bounds checking???". Well, right now we're throwing an IndexOutOfRangeExpcetion. Later we might want to do something else like output to the console or some other more complicated reporting mechanism. When we do that, we'll need to update our ExpectedException tests a bit, but that's no big deal either.

As practice, you may want to try implementing the other tests from the previous blog post's test list using the pattern above. Just remember, Don't Repeat Yourself .

Discussion: showing all 0 messages

(No messages)

>

Post a new message:

---