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

Enhance Security and Prevent Unauthorized Access: Refuse 'password' as Custom shortURL, Protect Deletion of Key, and Skip 'password' in localStorage #6

Closed
wants to merge 3 commits into from

Conversation

nazdridoy
Copy link

Pull Request Description

Refuse 'password" as Custom shortURL

  • Introduces a check to prevent the use of "password" as a custom shortURL, enhancing security.

Do not accecpt any del cmd for "password" !IMPORTANT!

  • Implements a critical security fix to disallow unauthorized deletion of the "password" key. Previously, a malicious actor could potentially forge a POST request, leading to a 500 Internal Server Error with the appropriate error message.

Skip adding the entry if the key is "password" (ref commit: 05353cf)

  • Enhances the security of the website by preventing the unintentional exposure of sensitive information stored in the browser's localStorage. Previously, the application would load and list all entries from the localStorage, including entries with keys like "password." This posed a security risk, allowing users to manually add a "password" key to the localStorage, and subsequently, deleting it would disrupt the site's functionality. The updated logic skips adding any entry with the key "password," mitigating the risk of accidental or intentional deletion of this sensitive information.

Changes

  • Mainly includes security enhancements and crucial fixes to prevent unauthorized access and maintain the stability of the website.

Additional Information

  • Showing with 29 additions and 10 deletions across main.js and worker.js.
  • Commits authored by @nazdridoy.

""""""""""

"password" is a protected key
Prevent unauthorized deletion of the "password" key by checking for a 'del' command targeting it. Previously, a malicious actor could potentially forge a POST request with the 'del' command for the "password" key, leading to the website becoming unreachable. This commit addresses this security vulnerability by explicitly disallowing the deletion of the "password" key. The added check ensures that any attempt to delete this key results in a 500 Internal Server Error response with the appropriate error message. This change is crucial for maintaining the security and accessibility of the website. It is recommended to apply this fix to prevent unauthorized access to sensitive information.
Enhance the security of the website by preventing the unintentional exposure of sensitive information stored in the browser's localStorage. Previously, the application would load and list all entries from the localStorage, including entries with keys like "password." This behavior posed a security risk, allowing users to manually add a "password" key to the localStorage, and subsequently, deleting it would disrupt the site's functionality.

This commit introduces a crucial improvement. Now, when loading localStorage entries, any entry with the key "password" is explicitly skipped, mitigating the risk of accidental or intentional deletion of this sensitive information. The revised logic ensures that only valid entries, excluding "password," are processed and displayed. This change contributes to a more secure handling of localStorage data and prevents potential disruptions caused by inadvertent key deletions.
@crazypeace
Copy link
Owner

Hi,
Thanks for your PR.
Somehow I didn't get any notification from Github about this, so I missed, sorry.
Recently, I made a function of load KV. Then there is possiblity to display delete button for 'password'. So I set a protect_keylist to prevent delete 'password'.

const protect_keylist = [
  "password",
]

About the display 'password', I think:

  1. The backend of this project is more like a general KV database, so I just let 'password' displayed as other records.
  2. The one who can see the 'password' already know it.
  3. I set a protection for deleting it.

So I think it's enough, thus I don't set a filter to hide it from the operation UI.
Whatever, that's only my personal idea. I'm not very sure that I'm correct. I may change my mind in the future. But just now I made this decision and set it this way.

@crazypeace
Copy link
Owner

Now I set this:
At the backend, protect the protect_keylist from add, del, qry cmd.
And hide this in qryall cmd.
af380db

@nazdridoy
Copy link
Author

Now I set this: At the backend, protect the protect_keylist from add, del, qry cmd. And hide this in qryall cmd. af380db

well done @crazypeace .So far, these issuses seem resolved. I'm closing this PR.

@nazdridoy nazdridoy closed this Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants