nilsteampassnet / TeamPass

Collaborative Passwords Manager
https://www.teampass.net
1.65k stars 537 forks source link

Password Items Labels do not preserve the '\' character #1445

Closed 46Wil closed 8 years ago

46Wil commented 8 years ago
### Steps to reproduce 1. **\+ Add item** 2. Enter a _Label :_ name, be sure to include the '\' character. 3. Enter a password that meets the password requirements. 4. Select **Save** ### Expected behaviour When a new password item is saved that contains the '\' character the PW list should display the label with the '\' character. ### Actual behaviour Instead it actually saves the '\' character as \. The '\' character is preserved in usernames, but not for the label. ### Server configuration **Operating system**: Teampass Server: CentOS 7 **Web server:** Apache/2.4/6 (CentOS) **Database:** 5.5.50-MariaDB **PHP version:** 5.6.24 **Teampass version:** 2.1.26 (15) **Updated from an older Teampass or fresh install:** ### Client configuration Chrome: 52.0.2743.116 m (64-bit) **Operating system:** Client: Windows 10 ### Logs #### Web server error log ``` Insert your webserver log here ``` #### Firebug log ([How to?](http://teampass.net/2014-02-09-how-to-communicate-an-error-log)) ``` Insert the Firebug log here ``` P.S. Behavior is misspelled above. Might want to correct the template.
qubez commented 8 years ago

The github bug report system seems to have stripped out the character in question. It only shows the backslash character ( \ \ Reverse solidus (backslash).

Can you describe the character, perhaps referring to the code number from: http://www.december.com/html/spec/codes.html or http://www.ebyte.it/library/educards/html/HTMLEntitiesAndGlyphs.html

qubez commented 8 years ago

meekrodb in TeamPass is several commits behind that project. There are several changes worth merging to the customized version of db.class.php in teampass, like https://github.com/SergeyTsalkov/meekrodb/commit/da46a1eacc683b34fcbba069c1f25f6930fbb9c9

Some deal with string encoding and decoding, and may correctly store protected characters, fixing bugs like this. Unit tests would need to confirm that no existing passwords are ever returned incorrectly.

The error logging hacks inserted into db.class.php could be moved into a parent wrapper function, so that meekrodb can stay updated.

46Wil commented 8 years ago

That is correct, it is the backslash, "\ \ Reverse solidus (backslash)", but it only has issues on the Label. It doesn't have issues in the password or in the username.

qubez commented 8 years ago

Putting aaaa\aaaa into the label field stores this in the database: aaaa\aaaa - however bbbb\bbbb in the description field stores bbbb\bbbb in the database.

Replacing the database label with one encoded like the description fixes everywhere except when editing the label, which shows semi-sanitized output - it removes amp; but not the #92;

The problem is caused by inconsistent encoding and decoding, specifically double-sanitizing html entities. Example is line 387 in sources/items.queries.php: $label = noHTML(htmlspecialchars_decode($dataReceived['label']));

It calls a teampass function noHTML in main.functions, that does: return htmlspecialchars($input, ENT_QUOTES | ENT_HTML5, $encoding);

The doctype should be ENT_XHTML, and the fourth parameter double_encode should be set to false, for starters. Elsewhere reversing the encoding is done inconsistently.

I'll see if i can work up a fix that will be compatible with any previously stored labels (or at least not make them worse).

A backslash in the URL field is even more misbehaved, giving a User not allowed to access this folder! error on creation or a _ERR_JSONFORMAT ERROR (JSON is broken)!!!!! on edit.

nilsteampassnet commented 8 years ago

The doctype should be ENT_XHTML, and the fourth parameter double_encode should be set to false, for starters. Elsewhere reversing the encoding is done inconsistently.

Yes absolutely, thank you for this tip.

I really appreciate all the help you provide on this project.