Mass-Assignment Vulnerability in updateProject

[Nixxx19]

p5.js version

latest

What is your operating system?

None

Web browser and version

all

Actual Behavior

The updateProject function in server/controllers/project.controller.js uses mass-assignment vulnerability by directly passing req.body to MongoDB's $set operator without field whitelisting. This allows authenticated users to overwrite any field in the project document, including critical fields like user (ownership), visibility, slug, and internal fields.

Location:

• File: server/controllers/project.controller.js
• Lines: 51-54

Current Code:

const updatedProject = await Project.findByIdAndUpdate(
  req.params.project_id,
  {
    $set: req.body  //  CRITICAL: No field whitelisting
  },
  {
    new: true,
    runValidators: true
  }
)

Expected Behavior

Only whitelisted fields should be updatable. The user field should never be modifiable through the update endpoint. Ownership changes should require a separate, properly secured endpoint.

Proposed Fix:

export async function updateProject(req, res) {
  try {
    const project = await Project.findById(req.params.project_id).exec();
    if (!project) {
      res.status(404).send({
        success: false,
        message: 'Project with that id does not exist.'
      });
      return;
    }
    if (!project.user.equals(req.user._id)) {
      res.status(403).send({
        success: false,
        message: 'Session does not match owner of project.'
      });
      return;
    }
    if (
      req.body.updatedAt &&
      isAfter(new Date(project.updatedAt), new Date(req.body.updatedAt))
    ) {
      res.status(409).send({
        success: false,
        message: 'Attempted to save stale version of project.'
      });
      return;
    }

    //  FIX: Whitelist allowed fields
    const allowedFields = ['name', 'files', 'updatedAt'];
    const updateData = {};
    allowedFields.forEach(field => {
      if (req.body[field] !== undefined) {
        updateData[field] = req.body[field];
      }
    });

    const updatedProject = await Project.findByIdAndUpdate(
      req.params.project_id,
      { $set: updateData },
      {
        new: true,
        runValidators: true
      }
    )
      .populate('user', 'username')
      .exec();

    // ... rest of the function
  } catch (error) {
    console.error(error);
    res.status(500).json({ success: false });
  }
}

Steps to reproduce

1. Authenticate as a regular user (e.g., user1)
2. Create a project you own (or use an existing project you own)
3. Get the project ID from the response or URL
4. Send a PUT request to /editor/projects/:project_id with your session cookie and malicious payload:

   {
     "name": "Hacked Project",
     "user": "<another_user_id>",
     "visibility": "Public",
     "slug": "malicious-slug"
   }

5. Verify that the project ownership has been transferred to <another_user_id> by checking the response or querying the project
6. Verify that unauthorized fields like visibility, slug, and user can be modified even though they shouldn't be updatable through this endpoint

Note: The ownership check at line 34 only verifies you own the project BEFORE the update. Once you pass that check, $set: req.body overwrites ALL fields including user, allowing you to transfer your own project to another user or modify any other field.

[welcome]

Welcome! 👋 Thanks for opening your first issue here! And to ensure the community is able to respond to your issue, be sure to follow the issue template if you haven't already.

[Nixxx19]

Hi @raclim!

I'd love to work on this issue and submit the fix for it.

[raclim]

Thanks for flagging this @Nixxx19! Tagging @Iron-56 for any extra thoughts on this issue!

[Nixxx19]

Thanks for the tag, @raclim! I'm standing by for @Iron-56's thoughts.

I've already drafted a fix using a whitelisting approach to ensure only fields like name, files, and updatedAt are mutable. I'm also double-checking the Project model for other internal fields (like _id, createdAt, or user) to ensure we're strictly following the principle of least privilege and preventing any future mass-assignment risks.

Happy to adjust the allowedFields list based on your feedback!

[Iron-56]

The fix looks good!, can you also add visibility and isSaving to the whitelist? Also how about changing it to immutable field in the mongose schema?

[Nixxx19]

Thanks, @Iron-56!

I'll add visibility to the whitelist and mark user as immutable: true in the Project schema for defense-in-depth.

isSaving isn't in the Project schema (it's client-only Redux state), so I'll leave it out.

I'm thinking of also marking slug as immutable so the project URL can't be changed via updates, added tests for updateProject, including the whitelist (e.g. sending user in the body doesn't change ownership).

I have created the PR #3893 with the changes. Let me know what you think.