django.contrib.auth.views.login can leak session

[Yuanchen Zhu]

Suppose django.contrib.auth.views.login is configured at a certain url, say www.test.com/login.

Now if a client visits www.test.com/login (using GET) for the first time, "login" calls "set_test_cookie", and a session database record will be created. Suppose the client has cookie disabled, and it visits www.test.com/login again using GET, a differnt session database record will be created to store the test cookie. This process can go on and the session database will be filled with records containing single test cookies, allowing a DoS attack on the database.

I found this problem when I used the apache benchmark tool "ab" to benchmark the login page of my website. After making 10000 requests using "ab" three times in a row, I was really surprised that the session database now contains 30000 records, each only contains a single test cookie. This is done locally so these 30000 requests finish in < 5 minutes. Thus I imagine that someone with enough network bandwidth can force the creation of >3 million records in the session database of some django powered site in an hour.

I think the best way to fix the problem is to change the implementation of set_test_cookie so that no record is created in the database session. For example, it can just set a cookie based on some hash of the source ip of the request. Later on, can just test if the cookie is returned in a second request using only the source ip of the request.

I am kind of new to web programming, so can people comment on if this is indeed a valid problem/solution, before I try to write a patch.

[Yuanchen Zhu <yuanchen.zhu@…>]

Ah sorry my math is off. It should be 300k, not 3 million. But that is still a lot of leaked session records.

[Jacob]

set_test_cookie doesn't create a session; it's the session middleware that's creating sessions. There's really no way around this -- if you want to use sessions, you have to give a new session to any browser that doesn't have one. Removing this would break session handling in a major way, and wouldn't actually stop DDoS attacks -- anyone who's gonna hit your server 100 times/second can do damage in other ways, and you need to just detect that and banninate him.

[Yuanchen Zhu <yuanchen.zhu@…>]

Right. But the session middleware won't actually hit the database and create a record unless the cookie store is not empty. While I do want to use sessions, I don't want everyone who visits a simple login page, without even logging in, to modify my database, and I want to be able to maybe cache the login page as static content.

You're definitely right that anyone who's gonna hit the server 100 times/sec should just be banned, so I guess this is not anything like a security problem. However, it is still a minor performance bug, since any page that performs a simple cookie test requres hitting the database rather unnecessarily.

What I was proposing was not rewriting the session system, but just change how set_test_cookie and its brothers are implemented, so these test cookies don't hit the database. I will gladly pay the overhead when I am actually using the session system to store some meaningful cookie.

Do you think what I've described in my first post constitute a valid alternative implementation for set_test_cookie? If not, why?

[Jacob]

It's just far too brittle -- source IP doesn't work, nor does any other HTTP header. Why not just switch to a different session backend if you're concerned about sessions in the DB?

[Yuanchen Zhu <yuanchen.zhu@…>]

I see. Good point. Thanks.