English Portuguese

Security Unit Tests Are Important

When it comes to application security, the main topics covered when talking to a security engineer are Dynamic Application Security Testing (DAST), Static Application Security Testing (SAST), Threat Modeling, Manual Pentest and Code Review. However there is one more topic not so widely discussed, but extremely important as well: unit testing for security.

"(...) Unit tests are typically written and run by software developers to ensure that code meets its design and behaves as intended. (...)" Wikipedia.

So let's start with an example.

Context:

Here's the function responsible for creating the book:

    def create_book(author_id, title)
        # Create new instance of Book, the model
        # And associate to it the values passed
        # as arguments above
        @book = Book.new({
            author_id: author_id,
            title: title
        })

        # If the save operation is successful,
        # We return the whole book object
        # Otherwise we return an array of errors
        if @book.save
            return @book
        else
            return @book.errors
        end
    end

And a typical invocation of this function, which handles the user request, would be:

    def handle_user_request
        # Params retrieve values from the request
        # So if the user pass { author_id: 1, title: "Oh my god "}
        # In a POST request, for example, each could be retrieved
        # Using "params", as written below.
        # 
        # PS: If those arguments were not passed,
        # we'll consider an empty string instead
        author_id = params[:author_id] || ''
        title = params[:title] || ''

        # Input Validation for Author Id (digits only)
        unless author_id =~ /\A[0-9]{1,5}\z/
            puts "Author Id is not valid"
            return false
        end

        # Input Validation for Title (only allowed chars on regexp)
        unless title =~ /\A[a-zA-Z0-9 ]{1,50}\z/
            puts "Title has not allowed characters"
            return false
        end

        # The execution didn't stop above. It means that
        # Everything looks cool, so let's create the book
        create_book(author_id, title)
    end

So, let's get started. We will create a unit test for this piece of code and then discuss about a security unit test for it.

A simple unit test, to verify if this book is being persisted and if the attributes are being persisted correctly, would look like this:

    def test_create_book_successfully
        # Fake Data
        fake_author = 1
        fake_title = "My Left Foot"

        # Create the Book
        result = create_book(fake_author, fake_title)

        # Test Expectation
        expect(result).to be_an_instance_of(Book)    # Book object was returned
        expect(result.author_id).to eq(fake_author)  # Author was persisted correctly
        expect(result.title).to eq(fake_title)       # Title was persisted correctly
    end

One more example. A unit test that expects unique book titles:

    def test_create_book_unique_title
        # Fake Data
        fake_author = 1
        fake_title = "My Left Foot"

        # Create Books using the same data
        result1 = create_book(fake_author, fake_title)
        result2 = create_book(fake_author, fake_title)

        # The first book should be created as expected
        expect(result1).to be_an_instance_of(Book)

        # But the second attempt must result in an error
        # Because we are using the same title, so we expect
        # An Array, instead of a Book object
        expect(result2).to be_an_instance_of(Array)
    end

Ok, now we've seen an example of piece of code that creates a book and two unit tests for it.

But this code so far has a serious security bug ...

It doesn't check if the author is creating a book on behalf of the others or not. If he is, we've found a bypass to our business logic.

To prevent that we must add proper protection and write a test for it. Test Driven Development enthusiasts would say that such steps are in the wrong order. We should develop the test first and the code later. Actually it's better to call it a specification rather than a test, so it sounds more clear. But whatever, as long as you do it. Here's the test:

    def test_book_creation_business_logic_bypass
        # Fake Data
        current_user = User.find(2) # Load User 2 as current logged in user
        params = {
            author_id: 1,           # But set the User 1 as author
            title: "My Left Foot"
        }

        # Try to Create Book on behalf of User 1
        result = handle_user_request

        # The result must be an array, because an
        # Array will mean an error was triggered
        # According to the create_book() function
        expect(result).to be_an_instance_of(Array)
    end

And here's the piece of code that will perform the check that must be put on handle_user_request function:

    # Input Validation for Author Id (must match current_user.id)
    unless author_id == current_user.id
        puts "Author Id is not from the current user"
        return false
    end

In the end, the caller code would look like this:

    def handle_user_request
        # Params retrieve values from the request
        # So if the user pass { author_id: 1, title: "Oh my god "}
        # In a POST request, for example, each could be retrieved
        # Using "params", as written below.
        # 
        # PS: If those arguments were not passed,
        # we'll consider an empty string instead
        author_id = params[:author_id] || ''
        title = params[:title] || ''

        # Input Validation for Author Id (digits only)
        unless author_id =~ /\A[0-9]{1,5}\z/
            puts "Author Id is not valid"
            return false
        end

        # Input Validation for Author Id (must match current_user.id)
        unless author_id == current_user.id
            puts "Author Id is not from the current user"
            return false
        end

        # Input Validation for Title (only allowed chars on regexp)
        unless title =~ /\A[a-zA-Z0-9 ]{1,50}\z/
            puts "Title has not allowed characters"
            return false
        end

        # The execution didn't stop above. It means that
        # Everything looks cool, so let's create the book
        create_book(author_id, title)
    end

For this example, as we know what we are expecting, we could reject receiving "author_id" from request parameters and put directly "current_user.id" on create_book, so it may not be the best example.

On the other hand, I believe that I was able to pass onto you the concept of creating security unit tests for your applications. In this case we've created a test to check for business logic bypass, however that's only the tip of the iceberg.

You can write tests for other threats as well. In this case we could consider writing a test to test against SQL Injection attacks. As our code above already validate the user input on title and on author_id, we're covered, but there is no test for that.

A big help for this kind of test is the Big List of Naughty Strings, or SQL Injection Payloads. You may want to create a more generical test that could run those payloads against every possible route that receives user input instead of coding this same tests for each code unit. It's something for another post for sure, although I believe that you have grasped the idea.

Security unit tests can help a lot. They would have saved at least 15,000 USD for Facebook. They are not very well discussed but the scenario is changing.

That's all I have for today. I hope you have enjoyed :)


Share the knowledge :)

Share on Twitter Share on Facebook Share on Google Plus Share on LinkedIn Share on Hacker News