Mantis Bugtracker          
testlink.org

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0004342TestLinkUsers and Rightspublic2011-03-22 14:062012-09-01 19:59
ReporterJulian 
Assigned Tofman 
PriorityhighSeveritymajorReproducibilityalways
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version1.9.2 (2011 Q2 - bug fixing) 
Fixed in Version1.9.4 (2012 Q3 - bug fixing) 
Summary0004342: Security problem with multiple Testlink installations on the same server
Description1. Login to http://myserver.install01.net/demo [^] -> admin/admin
2. Edit URL in Browser to http://myserver.install01.net [^] and press Enter

-> i can see testlink qa data as admin and can edit the data. seems to be a real security problem which requires that i stop one installation of testlink on this server as soon as possible... to reproduce will need to have 2 local installations.
Additional InformationPassword for user admin on http://myserver.install01.net [^] is not the same as for admin on http://myserver.install01.net/demo [^]
TagsTO BE FIXED on 2.0
Database (MySQL,Postgres,etc)-
Browser
PHP Version
TestCaseID
QA Team - Task Workflow StatusREADY FOR TESTING
Attached Files? file icon secure-session-management-preventing-security-voids-web-applications_1594 [^] (352,658 bytes) 2011-04-09 15:18
txt file icon error_before_manual_migration.txt [^] (10,187 bytes) 2011-08-15 09:43 [Show Content]

- Relationships
has duplicate 0003146closed Simutanous 1.8.3 and 1.9.b2 cause wrong automatic login 
has duplicate 0004818closed automatic logon if multiple instances are is installed on a domain/server 

-  Notes
(0014217)
fman (administrator)
2011-03-22 22:52
edited on: 2011-04-10 16:30

may be we can find some idea here
http://en.wikipedia.org/wiki/Cross-site_request_forgery [^]
http://en.wikipedia.org/wiki/Session_fixation [^]
http://www.owasp.org/index.php/Session_Fixation [^]
http://www.owasp.org/index.php/Session_Management#Objective [^]
http://www.owasp.org/index.php/Testing_for_Session_Management [^]

Stupid idea:
if we save in session URL that have started session (or a hash) and found now is different (we are able to do this ?) we can invalidate session.


---------------------------------------------------
More info
http://advosys.ca/papers/web/60-form-tampering.html [^]

(0014527)
fman (administrator)
2011-04-09 16:19

I'm analizing Mantis solution, and going to use it's code.
this needs change on users table => cookiestring is saved
(0014531)
Julian (reporter)
2011-04-10 10:34

will you be able to add to 1.9 ? as this would require manual steps to add column to user table this could be made configurable (disabled per default) so confusion due to manual steps will be reduced... thanks for your work here!
(0014532)
fman (administrator)
2011-04-10 10:45

I will do work on 2.0 then if ok will try on 1.9.3, but will be a mandatory change to db => people will need to learn how to add a column to a table.
We can not have config for everything because is an overkill, nonsense and insane.
(0014533)
Julian (reporter)
2011-04-10 10:49

then we need some better way of letting people know that manual steps are required. i know many open source tools do not offer comfortable update procedures but maybe we find an easy approach to handle such minor scheme changes with minimum user action.

maybe login should be forbidden with a warning as long as db version does not fit required db version on code.
(0014534)
fman (administrator)
2011-04-10 10:58

Long time ago I've implemented the check, what we need to see / test if still work.
I do not plan to put big effort on this kind of minor upgrade because user with minimal cognitive capacity is able to understand how to add a column
You are free to work on this are if you want, checking first what already have, as usual
(0014613)
Julian (reporter)
2011-04-14 09:50

Please add a description of your change here and let us know how we can test.
(0014631)
fman (administrator)
2011-04-14 18:24

Added new column to users table cookie_string that will be setted for each user during user creation.
this value is setted on session and compared with value read from db if differente -> logout forced.

do TWO FRESH INSTALLS with HEAD CODE and you can test as you have done on demo

let me know if this is clear
solution is not still complete
(0014634)
Julian (reporter)
2011-04-14 19:24

let me know when you reached a state where tests make sense.
(0014635)
fman (administrator)
2011-04-14 19:53

test make sense now to check that your problem has been solved, in addition testing this do not require lot of time => please do this simple test.

Next step is block access to other pages knowing it's url.
(0014759)
Julian (reporter)
2011-04-28 11:41

I tried a quick test.

Seems to work but with current state on master i would not resolve the issue. When i login as admin i get minimum rights.. Think we should wait until changes from #4429 are more stable
(0015482)
Julian (reporter)
2011-07-08 10:41

If we backport to TL 1.9.4 we should think about the most simple way for users to upgrade DB.
(0015650)
fman (administrator)
2011-08-13 08:20
edited on: 2011-08-13 08:32

1. 1.9.4 - https://gitorious.org/testlink-ga/testlink-code/commit/f88e66013ac7f30a719e0faaa201c69dfa56237f [^]

2. https://gitorious.org/testlink-ga/testlink-code/commit/161ed7751decb94070bdc7a82e3436871082fa8f [^]

(0015653)
Julian (reporter)
2011-08-15 07:16
edited on: 2011-08-15 07:18

Tests done:

1. Login on Testlink 1.9.3. and do not log out. Open Testlink 1.9.4 (with this fix) in another tab -> Session is invalidated -> OK

2. New DB Installation
-> Admin user is created with a default cookie_string "21232f297a57a5a743894a0e4a801fc321232f297a57a5a743894a0e4a801fc3"
-> as this is insecure i wanted to check if cookie_string is changed when changing password of admin user -> problem 3.

3. Change Password for admin user -> "You can't use an empty Email address!" even though i entered a valid mail adress -> FAIL

4. No migration procedure yet exists.

5. Config Parameter is unclear -> is there an alternative to "TESTLINK_USER_AUTH_COOKIE" :

/**
 * Taken from Mantis to implement better login security, and solve
 * TICKET 4342
 */
$tlCfg->auth_cookie = "TESTLINK_USER_AUTH_COOKIE";

(0015655)
Julian (reporter)
2011-08-15 08:12

>> 3.
>need to check I've touched nothing.
>can you detail here email you have used ?
>(just tested with admin.tl@kil.com and worked on my install)
i used the same email: admin.tl@kil.com - same error. Issue seems to be only reproducable after fresh db installation -> email is empty for admin as default. If e-mail has been set i can also change the password without problems.

>> 4. No migration
>you have to use same migration procedure used for Req. spec revisions
oh did not realize it was already added when you merged the req spec revs. cool.

>> 5.
>This thing is something user has not mess => see no need to change name.
Why show it on config if it is not configurable?
-> Better place might be const.inc.php ?
(0015656)
Julian (reporter)
2011-08-15 08:32
edited on: 2011-08-15 08:33

another question araised when i checked the migration code:

UPDATE /*prefix*/users SET cookie_string=MD5(login);

does this mean cookie_string is always the md5 hashed login name?

-> If same login exists on 2 different databases the cookie_string will be the same on both databases?

(0015657)
fman (administrator)
2011-08-15 08:45

I've found this on mantis administrator manual but do not understand
==========================================================================
WARNING: A DEFAULT ADMINISTRATOR level account is created. The account
            name and password are administrator / root. Use this
            when you first login to MantisBT. Immediately go to
            Manage and create at least one administrator level
            account. Immediately after that DISABLE or DELETE the
            administrator account. You can recreate it but you
            should delete the account to prevent the cookie_string
            from being used to trick the package. It would be even
            better to rename the account or delete it permanently.
            REMEMBER: After setting up the package, REMOVE the
            default administrator account.
==========================================================================

>> UPDATE /*prefix*/users SET cookie_string=MD5(login);
we need to be sure this create an string is unique on DB during migration.
We can see if we can add a random piece.

I'm working on changing (as done in Mantis) cookie_string when password is changed if new password if different from old one
(0015658)
Julian (reporter)
2011-08-15 08:49
edited on: 2011-08-15 08:50

ok i understand the warning:

They know that they create a default cookie_string which is the same for all mantis installations -> insecure as this will allow misuse existing administrator session on other installations.

If the user is deleted the default cookie_string is gone with it.
The newly created user with admin rights gets a cookie_string created the way you do generate cookie_string:
    function auth_generate_cookie_string()
    {
        $t_val = mt_rand( 0, mt_getrandmax() ) + mt_rand( 0, mt_getrandmax() );
        $t_val = md5( $t_val ) . md5( time() );
        return $t_val;
    }

if changing the password will also change cookie_string i do not thing deleting/disabling the default admin user is necessary. Maybe documentation problem on mantis... otherwise i do not really understand the problem

(0015659)
fman (administrator)
2011-08-15 09:04

will use:
UPDATE /*prefix*/users SET cookie_string=CONCAT(MD5(RAND()),MD5(login));
(0015660)
fman (administrator)
2011-08-15 09:09

Please apply this and retest
https://gitorious.org/testlink-ga/testlink-code/commit/0991d2bb8aa6cdf4a9b755156f9623b99210bda3 [^]


IMPORTANT NOTICE
this has to be merged on head
(0015661)
fman (administrator)
2011-08-15 09:23

Got error regarding empty email.
Our lazy approach to development SUCKS!.

when you try to change password system do also checks on email, and you have (seems) two different forms.
Solution:
step ONE fill in admin email and save it
step TWO now you can change password.

may be we can assign task to improve this to Bruno.
what do you think ?
(0015662)
Julian (reporter)
2011-08-15 09:28

I see. Sure let him know :)

Will check your latest changes soon.

Isnt it possible to create admin user cookie_string the same way it is done on migration on testlink_create_default_data.sql ?
(0015663)
fman (administrator)
2011-08-15 09:29

apply also this
https://gitorious.org/testlink-ga/testlink-code/commit/e35656bceb3f2b1a8124aa2a5ef115ffea786480 [^]

to have admin cookie_string generated as you requested
(0015664)
Julian (reporter)
2011-08-15 09:43
edited on: 2011-08-15 09:45

1. Did 2 fresh installations in a row.
-> cookie_string is different for both admin users. -> OK

2. Migrated DB 1.4
-> cookie_string is differs now -> OK
BUT: If DB is 1.4 and you open login page "DB Access Error - debug_print_backtrace() OUTPUT START" comes up -> will attach -> error is: Unknown column 'cookie_string' in 'field list'

3. After changing password -> cookie_string is also changed -> OK

(0017302)
fman (administrator)
2012-09-01 19:59

1.9.4 released

- Issue History
Date Modified Username Field Change
2011-03-22 14:06 Julian New Issue
2011-03-22 22:52 fman Note Added: 0014217
2011-03-22 22:53 fman Note Edited: 0014217 View Revisions
2011-03-22 22:57 fman Note Edited: 0014217 View Revisions
2011-03-22 22:58 fman Note Edited: 0014217 View Revisions
2011-03-22 23:01 fman Note Edited: 0014217 View Revisions
2011-03-22 23:01 fman Note Edited: 0014217 View Revisions
2011-04-09 15:18 fman File Added: secure-session-management-preventing-security-voids-web-applications_1594
2011-04-09 16:19 fman Note Added: 0014527
2011-04-10 10:34 Julian Note Added: 0014531
2011-04-10 10:45 fman Note Added: 0014532
2011-04-10 10:49 Julian Note Added: 0014533
2011-04-10 10:58 fman Note Added: 0014534
2011-04-10 16:30 fman Note Edited: 0014217 View Revisions
2011-04-14 09:50 Julian Note Added: 0014613
2011-04-14 18:24 fman Note Added: 0014631
2011-04-14 19:24 Julian Note Added: 0014634
2011-04-14 19:53 fman Note Added: 0014635
2011-04-28 11:41 Julian Note Added: 0014759
2011-06-07 09:19 Julian Assigned To => fman
2011-06-07 09:19 Julian Status new => assigned
2011-07-08 10:41 Julian Note Added: 0015482
2011-08-13 08:20 fman Note Added: 0015650
2011-08-13 08:32 fman Note Edited: 0015650 View Revisions
2011-08-15 07:16 Julian Note Added: 0015653
2011-08-15 07:18 Julian Note Edited: 0015653 View Revisions
2011-08-15 08:12 Julian Note Added: 0015655
2011-08-15 08:32 Julian Note Added: 0015656
2011-08-15 08:33 Julian Note Edited: 0015656 View Revisions
2011-08-15 08:45 fman Note Added: 0015657
2011-08-15 08:49 Julian Note Added: 0015658
2011-08-15 08:50 Julian Note Edited: 0015658 View Revisions
2011-08-15 09:04 fman Note Added: 0015659
2011-08-15 09:09 fman Note Added: 0015660
2011-08-15 09:23 fman Note Added: 0015661
2011-08-15 09:28 Julian Note Added: 0015662
2011-08-15 09:29 fman Note Added: 0015663
2011-08-15 09:43 Julian Note Added: 0015664
2011-08-15 09:43 Julian File Added: error_before_manual_migration.txt
2011-08-15 09:45 Julian Note Edited: 0015664 View Revisions
2011-08-17 21:28 fman Tag Attached: TO BE FIXED on 2.0
2011-09-03 14:34 fman Tag Detached: TO BE FIXED on 2.0
2011-09-03 14:40 fman Tag Attached: TO BE FIXED on 2.0
2011-11-23 15:31 Julian Relationship added has duplicate 0003146
2011-11-23 15:32 Julian Relationship added has duplicate 0004818
2012-04-14 17:44 fman Status assigned => work in progress
2012-08-07 14:40 fman Task Workflow Status => TBD
2012-08-07 14:40 fman Description Updated View Revisions
2012-08-07 14:40 fman Additional Information Updated View Revisions
2012-08-07 14:40 fman Additional Information Updated View Revisions
2012-08-07 14:42 fman View Status private => public
2012-08-07 14:42 fman Status work in progress => assigned
2012-08-07 14:42 fman Task Workflow Status TBD => READY FOR TESTING
2012-08-07 14:42 fman Status assigned => resolved
2012-08-07 14:42 fman Fixed in Version => 1.9.4 (2012 Q3 - bug fixing)
2012-08-07 14:42 fman Resolution open => fixed
2012-08-08 09:14 fman Bug Revision Dropped: Description: 0001928
2012-08-08 09:15 fman Bug Revision Dropped: Additional Information: 0001930
2012-09-01 19:59 fman Note Added: 0017302
2012-09-01 19:59 fman Status resolved => closed



Copyright © 2000 - 2018 MantisBT Team
Powered by Mantis Bugtracker