🏆Capstone project: Close the Server-Side Holes

You have eleven red tests and a server whose weaknesses you have not yet seen in the code. This lesson closes the first nine. They all live on the server’s side: in reading_server.py, signing.py, and passwords.py. Findings 10 and 11 are about requests from other pages, and they wait for the next lesson.

The method is the regression loop from Python in Practice, one finding at a time:

1. Run one test and read its failure.

2. Find the code the failure points at.

3. Change it.

4. Run all the tests, to see that the one you wanted turned green and that nothing else turned red.

python -m pytest tests/test_attacks.py -q -k form_cannot_delete

-k selects the tests whose names contain that text. Without it, all eleven run.

From a symptom to a line

Here is the whole path once, for finding 7. The other eight are yours.

The symptom is “Another reader can delete my book with the form on the page.” Your test logs Ben in, posts the form for book 1 with Ben’s own cookie and form token, and asserts a 404. It fails:

E       assert 303 == 404

A 303 is the answer to a delete that worked. So the request passed every check on its way: the method, the Origin, the session, the form, the form token, and the id. The contract gives that order in its route section, and the next check in it is the owner.

The form posts to /books/delete. In route(), that branch finds the session and calls delete_from_form(). Read that function against Chapter 6’s rule: a request is refused unless a rule allows it. The function looks the book up, checks that the id exists, and removes it. It never asks who owns the book. DELETE /books/1 with a token asks may_change() through item_refusal(). The form is a second path to the same delete, and it skipped the question. That was the lesson of Chapter 6: an owner rule checked on one path and not on another is not a rule.

The fix is one condition, and the function to call already exists.

Notice what found the line. It was not a search for “bug”. It was the test’s failure, the order of checks in the contract, and one rule you already know. Each of the other findings yields to the same three things. The rule is in the chapter that first closed the finding, and the function you wrote then is usually still in the file, just no longer called or no longer complete.

Do not close the door on everyone

A finding is closed when its attack is refused and every honest request still works. The second half is easy to lose. Take finding 9. This change makes its test pass at once:

def safe_cover_path(folder, name):
    return None

No name can leave the folder now. No cover is served either, and SECURITY.md says under “Must still work” that GET /covers/kindred.txt returns the cover. Check my work reads that list too. It logs readers in and counts the password hashes, keeps a session alive by using it every half hour, deletes a reader’s own book with their own form token, adds a book with all five fields, and asks for a cover. A fix that refuses any of those is reported as plainly as an open finding.

The same care applies to the status a refusal gets. The contract answers 404 for a book that is not the caller’s, so that the answer does not confirm the id, and 422 for a body that names a field the client may not set. A fix that answers 403, or that quietly drops the field, closes the hole and breaks the contract.

Your task

Close findings 1 to 9, in the contract’s order. After each one, run all the tests:

python -m pytest tests/test_attacks.py -q

The count of failures should go down by one each time:

If the count drops by two, read the diff again: one change closed something you did not mean to touch, and you want to know why. If it does not drop, the failure message of that one test is the place to look.

Two hints about where things are. Findings 1 and 2 are not in reading_server.py: a login’s work is done in passwords.py, and a tag is compared in signing.py. Findings 5 and 6 are in signing.py too. Everything else is in reading_server.py.

Then press Check my work. It calls your functions and handle() with users, books, a key, and clock times you have not seen, once for each of the nine findings, and it does not look at how a fix is written, only at what the code does. It checks every row of “Must still work”. It runs your own tests for findings 1 to 9 against your files. And it runs its own copy of the attack tests you wrote in Chapters 1 to 10, because the test file in this chapter started empty and nothing else guards those earlier rows.