Skip to content

Potential vulnerability patches#88

Open
dankmeme01 wants to merge 3 commits into
mainfrom
vuln-fixes
Open

Potential vulnerability patches#88
dankmeme01 wants to merge 3 commits into
mainfrom
vuln-fixes

Conversation

@dankmeme01

Copy link
Copy Markdown
Member

If you can't beat them, join them.. so this is an AI powered PR to fix potential security holes & DoS vectors in the index :D

(note: AI only used to find, not fix)

changes:

  • GitHub token no longer logged in error messages
  • image::load_from_memory replaced with a safer version that respects limits
  • Download URL length is now capped to 1024 chars to avoid anything funny
  • Updates endpoint accepts max 200 mods (client already respects this limit)
  • CORS changed to not allow credentials (pls verify this change is correct)
  • Some limits (like per_page values) are now clamped
  • Order of operations is changed in one place to avoid borrowing a pooled DB connection and keeping it idle for a period of time (e.g. during a mod download)
  • LocalStorage changed to not allow path traversal (future proofing, impossible in current code)
  • Zip file reading changed to call take and not read more bytes than advertised in the file metadata. I don't know if this is a true vulnerability, but after a quick read of zip's source code, I could not find it clearly enforcing the limit, so this validation might not be pointless

non-security changes:

  • Why was the GithubClient struct created on demand each time? Why was a reqwest::Client created on demand on every operation in that client? We may never know.
  • .. plus use the shared http client in a few more places

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.

1 participant