|
From: <no...@so...> - 2002-06-03 00:40:39
|
Patches item #563700, was opened at 2002-06-03 00:39 You can respond by visiting: http://sourceforge.net/tracker/?func=detail&atid=403613&aid=563700&group_id=31885 Category: None Group: None Status: Open Resolution: None Priority: 5 Submitted By: Giancarlo Pinerolo (pingus) Assigned to: Nobody/Anonymous (nobody) Summary: Block user supplied session id and mode Initial Comment: This patch is instead of my previous one. Now I use to check if the sessio exists. It passed my tests. The creation of a new session is now dependent on a class variable: block_bogus_sid = true You ca always have it work as before by subclassing session in local.inc, and I suggest to use this subclas only in places you ca control it.. This was a gravious hole in PHPLIB, and PHP btw: let people force 'get' mode and create whatever session they like is a great security risk, a malware. ---------------------------------------------------------------------- >Comment By: Giancarlo Pinerolo (pingus) Date: 2002-06-03 00:40 Message: Logged In: YES user_id=163488 I've tried it, but things should be changed since that, because now it works only in cookie mode. I base myself on 4 tests, 2 with cookie enabled and two without. I use Netscape and while enabling/disabling cookies, I keep 'warn me when a cookie is issued', so I can see them. Between each test, I close the browser and have a script that erases all cookies. For the 4 tests I use the latest php-lib-stable from the cvs, opening the 'simple page' provided in the distribution. In both modes (cookies enabled and disabled) I try the bare URL (index.php3) and one with a fake session appended. Every new test with the fake session is done modifying its value, eg adding a letter or a number, so that the session is not eventually found, as it should be if provided by the user. In The index.php3 I echo the session sid as a check. the test and the expected results are 1) clean cookies. Start browser. Open index.php3, cookies enabled. This should work pretty straight anyway, URL with session the very first time, counter increases. I should see a single warn for a cookie left. 2) clean cookies. restart browser, index.php3?Example_Session=byebye: -I should see a cookie left on the browser (get has not to be forced by user) -its value must NOT be 'byebye', (sid has not to be forced by user) -the page is redirected to itself with the correct session in the UrL too (the 302 cookie test needs that) -the counter increases 3)Restart browser, disable cookies. Open index.php3 -I should be redirected to index.php3 with a new session written in the URL. Further reload increase counter 4)Cookies disabled. Open index.php3?Example_Session=byebye22 -I should be redirected to index.php3 with a correct session in the URL (sid has not to be forced by user). Further reloads increase counter. Now that test on the existence of a sid does not work in get mode for various reason. First because when the id is found in the url, even if it does not exists, you are never redirected to the newly generated url. By the code now, that situation occurs only if there no Session in the url. Before the 302 redirect, there is no freeze, so you'll never find a newly created session that has been redirected to itself with it in the URL. The trick is to $this->freeze before the 302. And not to be mislead by the presence of a session in the URL, leave a cookie in any case, get too. And a few other changes that I applied. Anyone interested can see the patch at the phplib project on sourceforge. The patched session.inc has an added session variable var block_bogus_sid = true; This commands all the behavior. At the moment it passes all 4 tests on true (blocking ON). On false (no blocking applied) test no 2 gets the fake cookie release twice. I'll check that. There's some overhead, a read at every page to see if session exists. My previous patch didn't have this, because it was based on the idea of 'signin' every 'original' session with a fingerprint, so that post-and create session would not have it. But I prefer this, is more clean. Also I've seen that the situate between the release_token and get_id functions is getting quite complicate. get_id was first supposed to be invoked after release_token, but now release token invokes get_id too. ---------------------------------------------------------------------- You can respond by visiting: http://sourceforge.net/tracker/?func=detail&atid=403613&aid=563700&group_id=31885 |
