Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Buffer overflow & session / form protection #571

Closed
juzna opened this issue Mar 1, 2012 · 8 comments
Closed

Buffer overflow & session / form protection #571

juzna opened this issue Mar 1, 2012 · 8 comments

Comments

@juzna
Copy link
Contributor

juzna commented Mar 1, 2012

Buffer overflow can cause an web app to stop working, if it depends on output buffering while sending cookies (or, if it starts session lazyly, adds protection to form, ...).

Output buffer is enabled in PHP by default and set to 4096B. Thus, you may "output" upto 4KB of content while still being able to send headers/cookies. However after this buffer has been filled up, it is flushed and you're not able to send headers/cookies anymore. If you do so, Nette\Http\Response::setCookie() will throw.

This behavior may cause problems if your app is rendering dynamic data before sending a cookie. It would all work from the beginning, but as the app is being used, the chunk of data grows until it fills the buffer, which leads to an exception.

Example

Imagine very simple app based on sandbox. It just displays posts from users, and has a form on the bottom. There is no reason not to addProtection on such form. But addProtection will initialize session, which in turn will send a cookie.

You start with zero posts so it all fits into output buffer. But as user add their posts, the buffer will eventually overflow and the session won't be able to get initialized.

This problem is very tedious in that sense, that you've got your session already started since you opened the web page early (before overflow). Thus, you won't see any problems. But as new people come, they will be affected by it. What becomes even more frustrating is that it will all still work in your browser (as you've got the session still opened).

@juzna
Copy link
Contributor Author

juzna commented Mar 1, 2012

I think it would be useful to trigger E_USER_NOTICE or E_STRICT if one tries to send a cookie while some data is already in the output buffer. This won't stop the app from working (won't throw), but will tell the user there may be a problem.

Or would such behavior break something else?

@juzna
Copy link
Contributor Author

juzna commented Mar 1, 2012

Btw this caused my web to break after @hosiplan tweeted a link to it ;)

@juzna
Copy link
Contributor Author

juzna commented Mar 1, 2012

Btw just to be clear, it's not a bug in Nette. One would have the same issue in plain PHP. But Nette can be useful in warning the programmer before the problem occurs.

@vrana
Copy link
Member

vrana commented Mar 8, 2012

I think that Sandbox should add a session start call (maybe commented out by default but with a note that you need this if you would like to add a protection to your form).

@juzna
Copy link
Contributor Author

juzna commented Mar 8, 2012

Sandbox has sesstion set to smart, which is kind of "when it's needed or already exists". This is good, as I dont want to create sessions for everyone for no reason.
Anyway if it's mentioned somewherer in the doc, who would read it? I guess most people would forget about it. Why not to show a notice if such bad practice happens? Or would it break working sites?

@fprochazka
Copy link
Contributor

I support this change. It makes sense to me. The framework should teach good practies.

@jasir
Copy link

jasir commented Mar 8, 2012

Yes, this is exactly bug I have experienced couple of days ago and such thing would saved my ass then.

@dg
Copy link
Member

dg commented Mar 30, 2013

Merged c13ed0e

@dg dg closed this as completed Mar 30, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants