In-Portal Issue Tracker

Welcome to the In-Portal Open Source CMS Issue Tracker! This is a central management / tracking tool for all types of tasks / issues / bugs for the In-Portal Project. Before reporting any issues, please make sure to read the Guide into Issue Tracker and How to Properly Test and Report Bugs!

Viewing Issue Advanced Details Jump to Notes ] Wiki ]  Related Changesets ] View Simple ] Issue History ] Print ]
ID Category Type Reproducibility Date Submitted Last Update
0001436 [In-Portal CMS] Security feature request N/A 2012-11-05 11:24 2012-12-13 02:46
Reporter alex View Status public Project Name In-Portal CMS
Assigned To alex Developer
Priority normal Resolution fixed Platform
Status resolved   OS
  OS Version
ETA none Fixed in Version 5.3.0-B1 Product Version 5.1.0
  Target Version 5.3.0 Product Build
Time EstimateNo estimate
Summary 0001436: Encrypt cookie stored at client
Description I've found an interesting article about mistrusting cookie values submitted by browser to web server - http://phpadvent.org/2011/bake-cookies-like-a-chef-by-michael-nitschinger.

That article explains in details how we can encode/hash cookie values to make sure that In-Portal did set these cookies and they were not faked by user, who wants to hack website.
We can use random string as password used to hash/encode cookies.
Steps To Reproduce
Additional Information
Tags No tags attached.
Reference https://groups.google.com/d/topic/in-portal-dev/qenm_MavpZc/discussion
Change Log Message Add cookie encryption
Estimate Points 1
Attached Files patch file icon encrypt_cookie_1436.patch [^] (4,936 bytes) 2012-12-07 12:26 [Show Content]
patch file icon encrypt_cookie_1436_v2.patch [^] (9,487 bytes) 2012-12-10 10:31 [Show Content]
patch file icon encrypt_cookie_1436_v3.patch [^] (10,159 bytes) 2012-12-11 07:42 [Show Content]
patch file icon encrypt_cookie_1436_v4.patch [^] (13,880 bytes) 2012-12-12 05:30 [Show Content]
patch file icon warning_on_cookie_decrypting.patch [^] (1,183 bytes) 2012-12-13 02:44 [Show Content]

- Relationships Relation Graph ] Dependency Graph ]
related to 0001435resolved (5.3.0)alex Generate random string at install 

-  Notes
User avatar (0005315)
alex (manager)
2012-12-05 03:33

1. create kCookieHasher class (from article, which link is given in task) in "core/kernel/utility/cookie_hasher.php" file and register it in kApplication::RegisterDefaultClasses method

2. make $secret parameter in constructor optional and when it's not given use RandomString system setting value in it's place (see 0001435 task)

3. in Session::SetCookie method create instance of kCookieHasher class (via kApplication::makeClass - non static method !!!) and use it to hash actual cookie value that will be given to "setcookie" function

4. in kHTTPQuery::AddAllVars method before actual cookie values will be added into the class pre-process $_COOKIE array (in local variable) and undo hashing for cookies, that have it

5. cookies, written via JavaScript won't be affected by this change in any way
User avatar (0005327)
erik (manager)
2012-12-07 12:27

Patch attached. Needs testing
User avatar (0005328)
alex (manager)
2012-12-07 12:41

1. I don't see code, that checks if cookie isn't actually encrypted and passes it through. Instead I see code that gives even unencrypted cookies to kCookieHasher class which might even result in php error.

2. Only cookie value is hashed (not cookie name), that results in same hash for 2 cookies with identical values but different names. This way attacker can get 1 cookie value and place it in another cookie value.

3. What about upgrade, does it work normally when RandomString is generated during upgrade and suddenly cookies becomes encrypted.

4. I see potential problem with passing through cookies, that are not encrypted. This way attacker could make hacked cookie look like JS made cookie and pass it through without a problem. I don't know about a solution yet, any ideas?
User avatar (0005329)
erik (manager)
2012-12-10 10:36

New patch version attached. Needs testing.

1. Added code to prevent PHP warnings in cookies decoding process.

2. Added cookie name to hashable string

3,4. Made empty cookie values if after RandomString these becomes undecryptable.
User avatar (0005331)
alex (manager)
2012-12-11 03:05

1. Class kCookieHasher does all encryption/decryption of cookies and it also need to store cookies names, that doesn't need to be decrypted
2. No upgrade script
3. Usage of "var" is deprecated, please use corresponding visibility operator (e.g. private/public/protected)
4. Plain text cookie list contains cookie names, that In-Portal:
- don't write itself (e.g. "__ut*", "debug_*", "send_*", "use_*"), but keep "debug_session_id" and add "debug_off" cookie
- are written through PHP and must be encrypted (e.g. "cookies_on", "save_username")
4a. exclude all cookie names, that are written through PHP (e.g. Session::SetCookie call)

'catalog_active_prefix', 'cookies_on', 'debug_fastfile', 'debug_host', 'debug_port', 'debug_jit', 'debug_session_id', 'debug_stop', 'original_url', 'save_username', 'send_debug_header', 'send_sess_end', 'start_debug', 'TreeExpandStatus', 'use_remote', '__utma', '__utmz', '__unam'
User avatar (0005335)
erik (manager)
2012-12-11 07:43

1. Fixed
2. Fixed
3. Fixed
4. Fixed

New patch attached. Needs testing
User avatar (0005338)
alex (manager)
2012-12-12 05:29

Worked in general, but some minor things (related to code readability and disposition) were adjusted.

Fixes:
======
1. fixed spelling errors
2. added comments to all class properties/methods
3. added more checks on user entered plain text cookie names
4. fixed used ecryption key size to match used cipher (only first 32 symbols of 64 symbol random string are used)
5. a warning was produced during old cookie decryption after random string change

Failed expectations:
====================
1. any of encrypted cookies (even empty one) is 132 symbols long (too much, isn't it? but maybe serialized string with a signature would have similar length)
2. ecrypted cookies are completely unreadable (was expected), so readinging them without working In-Portal installation isn't possible
User avatar (0005339)
alex (manager)
2012-12-12 05:34

Fix committed to 5.3.x branch. Commit Message:

Fixes 0001436: Encrypt cookie stored at client
User avatar (0005340)
alex (manager)
2012-12-13 02:46

Patch "warning_on_cookie_decrypting.patch" solves problem, when cookie with "53741477" value being decrypted instead of being skipped because it isn't actually encrypted.

- Related Changesets
In-Portal CMS: 5.3.x r15651
Timestamp: 2012-12-13 02:48:02
Author: alex
Details ] Diff ]
Bug 0001436: Encrypt cookie stored at client
1. fixing warning on decryption of non-encrypted cookie
mod - /in-portal/branches/5.3.x/core/kernel/utility/cookie_hasher.php Diff ] File ]
In-Portal CMS: 5.3.x r15650
Timestamp: 2012-12-12 05:34:23
Author: alex
Details ] Diff ]
Fixes 0001436: Encrypt cookie stored at client
mod - /in-portal/branches/5.3.x/core/install/english.lang Diff ] File ]
mod - /in-portal/branches/5.3.x/core/install/install_data.sql Diff ] File ]
mod - /in-portal/branches/5.3.x/core/install/upgrades.sql Diff ] File ]
mod - /in-portal/branches/5.3.x/core/kernel/application.php Diff ] File ]
mod - /in-portal/branches/5.3.x/core/kernel/session/session.php Diff ] File ]
add - /in-portal/branches/5.3.x/core/kernel/utility/cookie_hasher.php File ]
mod - /in-portal/branches/5.3.x/core/kernel/utility/http_query.php Diff ] File ]

- Issue History
Date Modified Username Field Change
2012-12-13 02:48 alex Changeset attached 5.3.x r15651
2012-12-13 02:46 alex Note Added: 0005340
2012-12-13 02:44 alex File Added: warning_on_cookie_decrypting.patch
2012-12-12 05:34 alex Changeset attached 5.3.x r15650
2012-12-12 05:34 alex Note Added: 0005339
2012-12-12 05:34 alex Status reviewed and tested => resolved
2012-12-12 05:34 alex Fixed in Version => 5.3.0-B1
2012-12-12 05:34 alex Resolution open => fixed
2012-12-12 05:30 alex File Added: encrypt_cookie_1436_v4.patch
2012-12-12 05:29 alex Note Added: 0005338
2012-12-12 05:29 alex Status needs testing => reviewed and tested
2012-12-11 07:43 erik Note Added: 0005335
2012-12-11 07:43 erik Assigned To erik => alex
2012-12-11 07:43 erik Status needs work => needs testing
2012-12-11 07:42 erik File Added: encrypt_cookie_1436_v3.patch
2012-12-11 03:05 alex Note Added: 0005331
2012-12-11 03:05 alex Assigned To alex => erik
2012-12-11 03:05 alex Status needs testing => needs work
2012-12-10 10:36 erik Note Added: 0005329
2012-12-10 10:36 erik Assigned To erik => alex
2012-12-10 10:36 erik Status needs work => needs testing
2012-12-10 10:31 erik File Added: encrypt_cookie_1436_v2.patch
2012-12-07 12:41 alex Note Added: 0005328
2012-12-07 12:41 alex Assigned To alex => erik
2012-12-07 12:41 alex Status needs testing => needs work
2012-12-07 12:27 erik Note Added: 0005327
2012-12-07 12:27 erik Assigned To => alex
2012-12-07 12:27 erik Developer => erik
2012-12-07 12:27 erik Status active => needs testing
2012-12-07 12:26 erik File Added: encrypt_cookie_1436.patch
2012-12-05 03:33 alex Note Added: 0005315
2012-11-05 11:24 alex Relationship added related to 0001435
2012-11-05 11:24 alex New Issue
2012-11-05 11:24 alex Reference => https://groups.google.com/d/topic/in-portal-dev/qenm_MavpZc/discussion
2012-11-05 11:24 alex Change Log Message => Add cookie encryption
2012-11-05 11:24 alex Estimate Points => 1



Web Development by Intechnic
In-Portal Open Source CMS
In-Portal Open Source CMS
Copyright © 2000 - 2009 MantisBT Group

Powered by Mantis Bugtracker