dailydotdev / daily

daily.dev is a professional network for developers to learn, collaborate, and grow together 👩🏽‍💻 👨‍💻
https://daily.dev
GNU Affero General Public License v3.0
18.07k stars 467 forks source link

🐛 BUG: Insufficient API Validation #1378

Open cr4shed opened 3 weeks ago

cr4shed commented 3 weeks ago

What went wrong? 🤔

Insufficient API data validation can cause unexpected behavior. The following mutations seem to bypass some frontend validation:

  1. UpdateUserProfile accepts a bio string exceeding the frontend maximum of 100.
  2. UpdateUserProfile accepts an experienceLevel string with any text in it, regardless of whether it appears in the predefined dropdown list.
  3. UpdateUserProfile accepts name being null, subsequent updates to the profile with name null does trigger validation however and the request fails.
  4. UpdateUserProfile accepts any string for image and will attempt to retrieve whatever is entered. For instance I can make my image an external gif (which seems to be unsupported by the frontend). This may enable persistent CSRF or XSS attacks. It does seem however that the Squad image validation does only accept an upload and does not accept links.
  5. UpdateMemberRole allows admins to change their own role.
  6. generateDevCardV2 allows users to change their theme regardless of whether they meet the reputation requirements.

Expected Behavior

The below is what I expected when sending these mutations:

  1. API validates bio and fails when it exceeds 100 characters.
  2. API validates experienceLevel and fails if it is not a predefined value.
  3. API validates name and does not accept null.
  4. API validates image and only accepts an upload.
  5. API validates that [target]memberId != [sendingMemberId] or [targetMemberRole] != admin to prevent admins from changing roles for other admins.
    1. API validates reputation requirements for themes.

Steps to Reproduce Issue

I am going to omit the repro steps to prevent abuse but I am happy to explain further if necessary.

Solution Proposed

Ensure the API validates all user input, ideally at all levels (database, API, client).

Environment

No response

Browsers

No response

OS

No response

Version of daily.dev

No response

Additional Context

Bio, experience level, null name, legendary DevCard with 10 reputation, and gif for image can be observed at https://app.daily.dev/cr4shed. I also have a Squad with no admin which I achieved by changing my own role to moderator https://dly.to/MJBSxmuqRBp.

Code of Conduct

idoshamun commented 3 weeks ago

Hi @cr4shed,

Thank you for this deep analysis, we're aware of that and will fix that soon