[ZBX-14758] Cookie names and values should be URI-encoded Created: 2018 Aug 22  Updated: 2024 Apr 10  Resolved: 2018 Nov 05

Status: Closed
Project: ZABBIX BUGS AND ISSUES
Component/s: Frontend (F)
Affects Version/s: 3.4.12, 4.0.0alpha9
Fix Version/s: 3.0.24rc1, 3.4.15rc1, 4.0.2rc1, 4.2.0alpha1, 4.2 (plan)

Type: Incident report Priority: Minor
Reporter: Fernando Schmitt Assignee: Ivo Kurzemnieks
Resolution: Fixed Votes: 0
Labels: cookie, encoding, frontend, javascript
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified
Environment:

all

Attachments: PNG File chrome.zabbix.cookies.png     Text File js_class.cookie.patch    
Team: Team B
Sprint: Sprint 41, Sprint 42, Sprint 43, Sprint 44, Sprint 45, Sprint 46, Nov 2018
Story Points: 0.5

 Description   

File js/class.cookie.js does not encode cookies' name or value. As a result, some cookies such as 'cb_items_10114_1' are set with values containing invalid characters such as commas: "31757,31827,31828".

Accessing Zabbix directly, via NGINX + PHP, there is no problem.

Some federation services, such as Microsoft ADFS, however, do check cookie compliance, and return a HTTP 500 error instead of allowing Zabbix frontend to load.

There is a simple fix to that, only file js/class.cookie.js is affected. I've attached a patch as an exemple. Tested and working (see screenshot with cookies properly encoded).

 Comments   
Comment by dimir [ 2018 Aug 23 ]

Nice catch and thank you for the patch!

Comment by Ivo Kurzemnieks [ 2018 Aug 30 ]

RESOLVED in svn://svn.zabbix.com/branches/dev/ZBX-14758

Comment by Fernando Schmitt [ 2018 Sep 07 ]

I think I know why it failed the test. In line 109 of the code in SVN for this issue you have:

else if (document.cookie.indexOf(name) != -1) {

But you should have:

else if (document.cookie.indexOf(encodeURIComponent(name)) != -1) {

Comment by Valdis Murzins [ 2018 Sep 07 ]

Exactly. But there is also a second one: in cookie.createArray() part length is counted before encoding, resulting in too long cookie value.

Comment by Fernando Schmitt [ 2018 Sep 07 ]

 I missed that one... If you ignore that the function enforces slicing the value string at commas (which is not required used by cookie.readArray), then you can fix like this (untested):

Line 59:

var list = encodeURIComponent(value.join(','));

Line 74:

tmp_index = list_part.lastIndexOf('%');

Lines 76-77 (move the '%' and whatever follows it to the next part):

list = list_part.substring(tmp_index) + list;
list_part = list_part.substring(0, tmp_index);

Line 80:

result = this.create(name + '_' + part, decodeURIComponent(list_part), days);

Function this.Create will then reencode the value... I know it's not the best solution 

Comment by Valdis Murzins [ 2018 Oct 12 ]

It was also decided to change separator from "," to "." when saving list of IDs.
It was done, because encoded comma takes a lot of cookie space, that could otherwise be used to fit more IDs in it.

Comment by Valdis Murzins [ 2018 Nov 05 ]

Fixed in:

  • 3.0.24rc1 r86439
  • 3.4.15rc1 r86441
  • 4.0.2rc1 r86445
  • 4.2.0alpha1 (trunk) r86447
Generated at Wed Dec 24 08:00:33 EET 2025 using Jira 10.3.13#10030013-sha1:56dd970ae30ebfeda3a697d25be1f6388b68a422.