search

sbondCo / Watcharr

Open source, self-hostable watched list for all your content (movies, tv series, anime, games) with user authentication, modern and clean UI and a very simple setup.
https://watcharr.app
MIT License
434 stars 21 forks source link

Possibly a bug? #649

Open yamerooo123 opened 4 days ago

yamerooo123 commented 4 days ago

Hello,

I'm a security research. I perform a security testing on open-source projects for free.

Bug

  1. Users can create a username with a space in it. For example "tester" and " tester". However, these two users are separate accounts. There is no security issue here.

Vulnerabilities

  1. Lack of expiry in token: It seems like JWT token has no expiration and considers a vulnerability. Since Change password function validates users from token. Someone can reuse the vulnerable token to change victim password to maintain persistence access. 2. Lack of CSRF in Change password function: Without CSRF protection, adversaries could exploit the lack of verification for state-changing requests (like changing a password). This could allow an attacker to trick a logged-in user into making a request to change their password by embedding a request in a malicious site or email.

Mitigation:

  1. I would suggest you to make the token to have an expiration 2. If possible, implement CSRF protection in functions that required authenticated users to interact.

If you need more information please let me know.

Have a nice day!

IRHM commented 4 days ago

Hey @yamerooo123 thanks for taking the time to dig in and find problems!

  1. That's a good point, I should probably make sure new accounts cant match existing ones (after trimmed spaces and not case sensitive).
  2. Lack of token exp: If I remember correctly, we don't issue a new token after change password, the user continues using their existing token. I see what you mean though, users might expect to be able to (optionally) expire all other tokens in use for their account. Quick research tells me token versioning is probably the way i'd want to fix/support this (so i dont forget: include a "version" in all tokens, if user wants to expire all tokens, increment this version (probs add version to db for comparisons), then auth with tokens that dont have the new version will fail.
  3. Lack of CSRF protection: We don't use any cookies and from my understanding, if you dont use cookies, you don't have to consider CSRF (since the browser cant automatically attribute any (auth) cookies with a request if you dont use them)? I'm not too smart about this stuff, I think the app could still be vulnerable to XSS attacks (if im right about the csrf thing). I will have to do more research on this (I would appreciate your thoughts on this!).
yamerooo123 commented 3 days ago

Ah, my bad, I mistook Session Storage for Cookies when inspecting. Yes, you're right, the web app doesn't need a CSRF token since it is session-based and not cookie-based. I will continue to find more bugs and vulnerabilities and report to you via this issue as soon as i found one.

IRHM commented 2 days ago

@yamerooo123 Thanks for making the video - would you be able to upload debug logs for the server of those requests going through?

IRHM commented 2 days ago

@yamerooo123 Do you have matrix (if so, are you able to message me at @irhm:matrix.org to discuss this further)

IRHM commented 2 days ago

@yamerooo123 I have hopefully resolved the issue you discovered in the next release coming out shortly.

I hope you don't mind, I deleted your comments as to not let any malicious people easy find out how to exploit the issues so easily.

If you are free to test this after the release I would be very greatful.

Thank you very much for your research into this! I should probably make a more secure way for discussing security issues in the future.

yamerooo123 commented 1 day ago

Yes. I would be glad to help you test with the new version.

You can use GitHub security tab to make a secure communication for security researchers to report vulnerabilities to prevent public disclosure as well.

In addition, your web app is bulletproof to SQL injection and actually is well implemented. Since your web app is heavily rely on API resources and sessions, I would make those the first priority when it comes to security issues.

Also, I have a small favor, I would like to request a CVE for this finding( only the recent one). CVE will help users to understand and advise them to upgrade to the latest version to patch the vulnerabilities. It is also quite common for many open-source softwares or applications to have CVE assigned too. Everything will be kept secret until the vulnerability has been patched.

Preliminarily, if I find more vulnerabilities, I will contact you via @irhm:matrix.org to prevent public disclosure.

Have a day!

IRHM commented 1 day ago

@yamerooo123 If it lets you make a CVE, please feel free, I hadn't thought of it, but if it helps alert people to upgrade then that sounds good (not sure if I have to make it?).

I had a look through the releases, it looks like the vulnerability has been present since v1.19.0 and should now be fixed in the latest release v1.44.0.

Thank you!

yamerooo123 commented 1 day ago

FINAL UPDATE

I THINK I FOUND THE CULPRIT! IT IS /DATA!

I undestand this folder is designed to be accessed only to high privileged users (sudo privilege) as it containing sensitive data which is why when JWT try to read JWT_SECRET, it couldn't retrieve because it lacks the privilege! Your code is solid but it has to do with Design Flaw . I retested it with jwt.io using a blank password. Now i couldn't craft JWT token anymore. This security flaw had me whole afternoon to figure it out. :satisfied:

You do not have to do it, as a security reseacher i will take care of it for you!