Ticket #81 (closed defect: fixed)

Opened 6 years ago

Last modified 15 months ago

Retrieving user-information (Session Leakage)

Reported by: dartar Owned by: JavaWoman
Priority: highest Milestone: 1.3.1
Component: core Version: 1.1.6.1
Severity: major Keywords: security, cookies, "multiple installations"
Cc:

Description

(Reported by GiorgosKontopoulos)

If you don't log out, then with a simple

echo "<PRE>_REQUEST =";print_r($_REQUEST)."</PRE>";

you can see the user's username and pass (md5'ed of course)

_REQUEST =Array
(
[skin] => xxxxxx.css
[PHPSESSID] => xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
[wikka_user_name] => xxxxxx
[wikka_pass] => xxxxxxxxxxxxxxxxxxxxxxxxxx
)

I think this is called a session leakage, anyone knows of a solution to this.

Perhaps a solution to this would be changing the name of the session that a particular wikka installation uses, The name could be a random number/word passed from md5 this way its unique to each wikka installation. Also changing the path that the session data are stored maybe helpful. (I have seen discussions on this I think on php.net session_name() or session_start() )

I don't really know the implications of this bug are (maybe its not even a bug), perhaps people can see the session data on shared hosts and that is really what concerns me. -GiorgosKontopoulos (look also at "Multiple wikis ... Login security hole?" later down the page)

Update There are more places this bug makes itself known. If one has accounts in two different Wikka wiki's sites then changing the skin in one will affect the active skin in the other Or if you log in/log out in one you find yourself logged in or logged out in the other.

tested it using 1) 2 Wikka engines, same server, using FF1.0.6, had same userName and pass on both behaves as stated above. 2) 2 Wikka engines, same server, using FF1.0.6, using userA on the one installation the other installation thinks I am userA but does not actually let me see pages only allowed to userA. 3) jsnx.wikka.com and my test installation using FF1.0.6, had same userName and pass on both it runs ok it seems.

Not a terrible bug but may have other implications still hidden. -GiorgosKontopoulos Oct. 27, 02:09:21 UTC

Related comments

Reuse of Username when multiple wikkas on single server

Not sure if this is truly a bug, but I have a number of wikka installations,. each in different (peer) directories on a server, each using different DB's. Now, each has a user named SeanOttey, and if I go log in at site A, then go to site B, I am shown as logged in, but with the other site's user info (i.e. email, password, settings). Potentially, this might create accessibility that an admin did not intend, yes? thanks! Sean.

Multiple wikka installations on one host: Login security hole?

Okay. Say that I've installed multiple seperate installations of wikka on one host. If I do a login on wikka A, I can also reach the pages of wikka B, whereas wikka B has an authentication table where the user account of wikka A does not exist! My guess is that this behaviour occurs because the login cookies are set with path root. And as long the login cookies exist Wikka doesn't care about authentication anymore?? -- JeroenJansen

  • Same problem. Is it possible to include in wikka.config.php a line in which you specify the default cookie prefix? I guess this could solve the problem. --YanB
    • Same problem for me as well. I've modified wikka.php to use different session names (add: session_name($wakkaConfigwakka_name?); just above the session_start(); ) and different cookies ("wikka_user_name@".$this->configwakka_name? and "wikka_pass@".$this->configwakka_name?) as my 2 wikkas do have have different wakka_names. Seems to behave as expected now. --OlivierPerron

Change History

Changed 6 years ago by minusf@…

i can only strongly agree with all above. this is clearly a security risk and bad design. and doesn't need too big changes to the codebase, so my quesiton is, why this hasn't been fixed yet?

1. a session_name() must be set 2. unless the user checks a "remember me" box i don't think any cookies should be set at all. i find this annoying. this should be on the login screen/user settings. 3. one cookie should be enough, stuff could be delimited with something in the content part.

Changed 6 years ago by minusf@…

one more thing, the cookies do not set the path. i have the wikka in a subdirectory, and cookie should be set only for this subdirectory. instead, it is set for all the server. this is not good.

Changed 6 years ago by minusf@…

hm, i didn't even realize that wikka did not work without cookies.... why is this? using $_SESSION, cookies are now things of the past. the only reason i see for using cookies is the "remember me" functionality. but otherwise?

Changed 6 years ago by NilsLindenberg

  • owner changed from unassigned to NilsLindenberg
  • status changed from new to assigned

Changed 6 years ago by NilsLindenberg

Ok, in fact these are two bugs.

1. session_start

We have to find something unique here. I do not think that the name of the wiki is a good idea. Because every visitor to your page will know it and, if he is on the same host, renaming his wiki to match your name is no problem. Perhaps we can take the url and strip everything except alphanumeric characters?

2. cookies

Same problem here, but perhaps we can use the wiki name and restrict the cookie path to the wikka directory?

Changed 6 years ago by JavaWoman

Here's a basic approach I'd like to see for starters:

1. define (as a CONSTANT) a "basic" cookie name - e.g. Wikka 2. extend the configuration to allow the definition of a cookie prefix 3. 1 + 2 should replace the name of the current PHPSESSID; by using a prefix you can have different installations on the same server still having a unique session cookie name 4. store the session cookie with this name AND with a full path 5. DO NOT store username and password in a cookie but store these (on the server) in the session itself

That should go a long way towards addressing this issue. I don't have any idea how much time this would involve though...

Going forward, we should also look at storing session data in a database table rather than temp files: that woudl be more flexibel in hosted installations and more scaleable as well.

Changed 6 years ago by JavaWoman

As to the aspect of Wikka not working without a cookie, that actually is *more* secure than not requiring one (provided we make the changes outined above). If you don't store the session id in a cookie you have to store it in the URL which on a shared machine can easily be found in the cache (or copied by mistake in an email): that would allow someone else to "take over" the session.

Changed 6 years ago by NilsLindenberg

We do now have (with [79]):

1. A basic cookie name, defined in wikka.php which gets used together with 2. a wiki_suffix to form a session_name

3. The same suffix is used for the cookies.

So if you want to use to different wikis on a server, you'll have to set two different wiki_suffix for both of them.

TODO: - store cookies with the full path - use one cookie to store the session_name and store user-name and pw in the session.

Using a database for session-storage should get another ticket!

Changed 6 years ago by DarTar

Nils - I'm testing your last code commit and the logout doesn't work anymore.

Changed 6 years ago by DarTar

false alarm - everything works fine again, I had messed up the URL parameters in usersettings.php, apologies, nothing to do with your cookie-related hack.

Changed 6 years ago by DarTar

Nils, how should we handle this ticket? If it has been fixed with a hack that will be rewritten in the next release, we might change its milestone to 1.1.6.3 or later for a better approach?

Changed 6 years ago by NilsLindenberg

  • milestone changed from 1.1.6.2 to 1.1.7

Not really rewritten, but extended. Beeing logged in into all wikis on the same server should not occur anymore if you change the wiki_suffix. But since not everything of jws basic approach has been coded, we might want to set a higher version, yes.

Changed 5 years ago by JavaWoman

  • keywords security, cookies added

Changed 5 years ago by JavaWoman

  • owner changed from NilsLindenberg to JavaWoman
  • status changed from assigned to new
  • milestone changed from 1.1.7.2 to 1.1.7

Changing milestone and reassigning to myself: implementing a proper cookie path is the very first thing that needs to be done now that we have a mechanism in place for installing several Wikkas on the same server sharing the code.

Even with different cookie names, not setting a proper cookie path and using "/" instead (even when Wikka is not installed in the root) is just plain wrong. Besides, "leaking" cookies across different applications is a security hole as well.

Making this a "child" issue of #545, and setting milestone to 1.1.7. It's so trivial to solve, there's no reason not to tackle this now. (In fact, should have been done for 1.1.6.3 already.) Priority and severity unchanged - they were right already!

Changed 5 years ago by JavaWoman

  • status changed from new to assigned

Changed 5 years ago by JavaWoman

This ticket has now been made a "child" ticket of #545

Changed 5 years ago by JavaWoman

  • keywords cookies, "multiple installations" added; cookies removed

Changed 5 years ago by JavaWoman

  • status changed from assigned to closed
  • resolution set to fixed

(In [668]) Better cookie handling

  • Ensures that the cookie path is always set to the path corresponding with Wikka's URL path - including session cookie (in wikka.php). Fixes #81
  • New method SetACookie() which supports setting both session cookies and (default) persistent cookies; old methods SetSessionCookie() and SetPersistentCookie are retained as wrapper functions for backwards compatibility (for now). Fixes #187
  • Enhanced DeleteCookie() to enable it to remove cookies with old (not suffixed) names and with a specific path
  • New method DeleteOldCookies() which removes old-name cookies as well as cookies with path set to root if the current path is not root; the Run() method now uses this method to clean up

Note that the code still contains some debug code; this will be cleaned up in a later phase.

refs #545

Changed 3 years ago by DarTar

  • milestone changed from 1.2 to 1.3

Retargeting to 1.3, this ticket has already been closed in trunk, from which 1.3 will be branched. Consider backporting urgent issues to 1.2.X

Changed 15 months ago by BrianKoontz

  • milestone changed from 1.3 to 1.3.1

Updated milestone to 1.3.1

Note: See TracTickets for help on using tickets.