Skip to content

Fix: Missing ownership check on PUT /api/Addresss/:id allows modifying other users' saved addresses - #366

Closed
WesternConcrete wants to merge 2 commits into
developfrom
devin/1789370009-address-update-owner-check-develop
Closed

WesternConcrete wants to merge 2 commits into
developfrom
devin/1789370009-address-update-owner-check-develop

Conversation

@WesternConcrete

@WesternConcrete WesternConcrete commented Sep 14, 2026

Copy link
Copy Markdown

Description

Finding: Missing ownership check on PUT /api/Addresss/:id allows modifying other users' saved addresses
Repo: COG-GTM/juice-shop
Fix: Route PUT /api/Addresss/:id to a new owner-scoped updateAddressById() handler in routes/address.ts instead of falling through to the generic finale-rest resource.

GET and DELETE /api/Addresss/:id already use handlers that query where: { id, UserId }, but PUT only ran security.appendUserId() and then let finale-rest update the row by id alone, so any authenticated user could overwrite another customer's delivery address. The new handler:

findOne({ where: { id: req.params.id, UserId: req.body.UserId } })  // 400 "Malicious activity detected." if null
address.update(pick(req.body, fullName|mobileNum|zipCode|streetAddress|city|state|country))  // 400 on validation error

Only fields present in the body are written, so partial updates behave as before. Test added in test/api/address.test.ts: another user's PUT returns 400 and the address is unchanged.

eslint and tsc --noEmit pass locally; the API test suite could not be run locally because frontend/dist is not built in this environment — relying on CI for test/api.

Resolved or fixed issue: none

AI Tool Disclosure

  • My contribution does not include any AI-generated content
  • My contribution includes AI-generated content, as disclosed below:
    • AI Tools: Devin
    • LLMs and versions: Devin (Cognition)
    • Prompts: Daily security sweep: substantiate and fix new vulnerabilities in COG-GTM/juice-shop

Affirmation

Link to Devin session: https://app.devin.ai/sessions/a749d64c0c0e49149bdaff3d2605b5c2
Open in Devin Desktop: https://app.devin.ai/desktop/session/a749d64c0c0e49149bdaff3d2605b5c2?variant=devin
Requested by: @WesternConcrete


Devin Review

Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Wes Convery <2wconvery@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

devin-ai-integration[bot]

This comment was marked as resolved.

Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Wes Convery <2wconvery@gmail.com>
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