#29 Block user supplied session id and mode

closed
nobody
None
5
2002-06-11
2002-06-03
No

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.

Discussion

  • Giancarlo Pinerolo

    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.

     
  • Giancarlo Pinerolo

    Logged In: YES
    user_id=163488

    Latest corrections:
    -block_bogus_sid=true works also when primary mode is 'get'
    -avoided double cookie setting when blocking is off

    None of these changes will llet you, ever, fall into
    fallback (get) mode when cookies are available.
    If you want to add that feature too, we should call it
    'forceback_mode' ;-)

    To force get, the standard way is to use an appropriate
    subclass, and not rely on 'forceback', which no more works.
    One hole less.

     
  • Giancarlo Pinerolo

    latst session.icn, blocks user-sid also on primary mode get

     
  • Giancarlo Pinerolo

    Logged In: YES
    user_id=163488

    Committed to the php-lib-stable branch, is now a standard,
    configurable, behavior

     
  • Giancarlo Pinerolo

    • status: open --> closed
     

Log in to post a comment.