Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Malformed custom fields can corrupt modification timestamps and password history #5636

Open
1 task done
bwbug opened this issue Jun 20, 2023 · 2 comments
Open
1 task done
Labels
browser Browser Extension bug desktop Desktop Application web

Comments

@bwbug
Copy link

bwbug commented Jun 20, 2023

Steps To Reproduce

  1. Open or create any vault item.
  2. Add at least two custom fields. To reproduce the password history corruption issue, these fields should be of the type "Hidden"; to only reproduce the corruption of the item modification timestamp, the custom fields may be of any type.
  3. Give the added custom fields the same field name.
  4. Give the first added custom field a unique field value.
  5. Give the second added custom field (and any subsequent added custom fields) a field value that differs from the unique value assigned to the first added custom field.
  6. Save the vault item.
  7. Make a note of the item's modification ("Updated") timestamp, and examine the item's Password History (if any).
  8. Open the vault item for editing, but do not make any changes to the item.
  9. Click "Save".
  10. Observe the item's modification ("Updated") timestamp, and examine the item's Password History.
  11. Optionally, repeat Steps 8-10.

Expected Result

Because no changes were made to the item before saving, the modification ("Updated") timestamp and Password History observed in Step 10 should not be changed, and should therefore match what had been observed in Step 7.

Actual Result

  • The item modification ("Updated") timestamp is updated when saving the unaltered item in Step 9, so that it no longer has the correct, original value from Step 7.

  • If the added custom fields are of the "Hidden" type, then the unchanged values of the second (and subsequent) custom fields are entered into the item's Password History, displacing older values. In particular, if there are 6 or more custom hidden fields that share the same name (and if the field value of the first of these hidden fields is unique), then the existing Password History is completely wiped out. If there are fewer than 6 custom hidden fields with a shared name (but at least 2 such fields, with field values that differ), then the Password History will be incrementally "erased" each time that the vault item is edited and saved.

Screenshots or Videos

A GIF video is available here.

Additional Context

Testing has been performed using the Desktop app and the Web Vault, but it is reasonable to infer that this issue exists in all client apps. Please add the tags for browser, web, etc.

A use-case that led to the discovery of this bug has been described in a recent thread in the Community Forum. A putative diagnosis of the issue is offered in one of the comments, and a follow-up comment provides a discussion of possible approaches to fixing the bug.

Operating System

Windows

Operating System Version

No response

Installation method

Direct Download (from bitwarden.com)

Build Version

Version 2023.5.0, Shell 24.1.1, Renderer 112.0.5615.50, Node 18.14.0, Architecture x64

Issue Tracking Info

  • I understand that work is tracked outside of Github. A PR will be linked to this issue should one be opened to address it, but Bitwarden doesn't use fields like "assigned", "milestone", or "project" to track progress.
@bwbug bwbug added bug desktop Desktop Application labels Jun 20, 2023
@SergeantConfused SergeantConfused added browser Browser Extension web labels Jun 26, 2023
@SergeantConfused
Copy link

Hi @bwbug,

Thank you for your report; I was able to reproduce this behaviour and I have flagged it to our engineering team.

If you wish to add any further information/screenshots/recordings etc., please feel free to do so at any time - our engineering team will be happy to review these.

Thank you again,

@bwbug
Copy link
Author

bwbug commented Jul 2, 2023

I just wanted to provide some suggestions and thoughts about potential approaches to fixing this issue.

  • Probably the simplest fix would be to prevent the user from creating custom fields with non-unique field names. I would strongly recommend against this approach, as there are valid use-cases for duplicating field names.

  • The best approach would be to modified the underlying data structure so that each field has a unique identifier (which does not have to be visible to the user), in which case the code should be able to unambiguously determine whether there have been any changes when the user clicks "Save". However, this solution approach would be work-intensive, and likely affect many parts of the codebase.

  • A possible interim solution that would partially fix the bug with limited impact on the codebase, would be to identify custom fields by the combination of field name, field type, and position relative to other custom fields that share the same name and type. In this way, custom fields that have the same name and type (but different values) would be detected as changed on Save only if the fields have been re-arranged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
browser Browser Extension bug desktop Desktop Application web
Projects
None yet
Development

No branches or pull requests

2 participants