Skip to content

macrozheng/mall mall-portal Privilege Escalation in POST /member/address/update/{id}: allows lateral overwrite via memberId to transfer address ownership #31

@Hwwg

Description

@Hwwg

macrozheng/mall mall-portal Privilege Escalation in POST /member/address/update/{id}: allows lateral overwrite via memberId to transfer address ownership

Contributors: Huang Weigang

1. Impact Scope

2. Vulnerable Endpoint

  • POST /member/address/update/{id} (Member receive address update API)

3. Code Analysis

  • Controller: mall-portal/src/main/java/com/macro/mall/portal/controller/UmsMemberReceiveAddressController.java

    • Route & method:
      • @RequestMapping(value = "/update/{id}", method = RequestMethod.POST)
      • public CommonResult update(@PathVariable Long id, @RequestBody UmsMemberReceiveAddress address) {
      • int count = memberReceiveAddressService.update(id, address);
      • return count > 0 ? CommonResult.success(count) : CommonResult.failed();
      • }
  • Service implementation: mall-portal/src/main/java/com/macro/mall/portal/service/impl/UmsMemberReceiveAddressServiceImpl.java

    • Key code (lines 40–60):
      • address.setId(null);
      • Build WHERE with current user: example.createCriteria().andMemberIdEqualTo(currentMember.getId()).andIdEqualTo(id);
      • Update default status if needed, then execute: return addressMapper.updateByExampleSelective(address, example);
  • Model: mall-mbg/src/main/java/com/macro/mall/model/UmsMemberReceiveAddress.java

    • Fields include: private Long id; private Long memberId; ...
  • Problem points:

    • memberId from request body is trusted and persisted. updateByExampleSelective updates non-null fields; if an attacker supplies memberId, it will be written.
    • The WHERE clause limits updates to “current user AND specified id” (memberId == currentMemberId AND id == {id}), but the update set allows changing memberId to any user, migrating the record to another account (lateral write).
    • Missing field-level permission control/whitelist for memberId and missing audit logging.

Reproduction

-- Prerequisites

  • Attacker has a valid login session (portal user).
  • Attacker knows their own address id (e.g., via /member/address/list).
  • Attacker knows or can enumerate the victim’s memberId.

-- Steps (lateral write: transfer address ownership to victim account)

  • Using the attacker account:
    • curl -X POST -H "Content-Type: application/json" "http://<host>/member/address/update/123" \ -d '{"memberId": 2, "defaultStatus": 1, "detailAddress": "Some Street"}'
  • Observation: API returns 200 OK; in DB, UmsMemberReceiveAddress(id=123) now has memberId=2 (victim account).
  • Verification:
    • Switch to the victim account and call GET /member/address/{id} or GET /member/address/list, the address appears under the victim; or query the database to confirm memberId changed.

4. Impact

  • Lateral write / data pollution
    • An attacker can move their address record into another account, causing ownership confusion and business risks (default address disorder, order shipping address mix-ups).
  • Account data integrity
    • Victim account is polluted by data not created by the victim, impacting downstream flows (ordering, delivery, after-sales).
  • Audit and accountability
    • No log records of “who changed address ownership”, hindering forensics and accountability.

5. Remediation

  • Server-side field masking & whitelist
    • In update(Long id, UmsMemberReceiveAddress address), explicitly block memberId updates, e.g.:
      • address.setMemberId(null); // or force address.setMemberId(currentMember.getId())
      • Ensure updateByExampleSelective cannot modify memberId.
  • Ownership validation & immutable field policy
    • Keep the WHERE ownership check (already present) and treat memberId as immutable; do not accept client-provided memberId.
  • Dedicated DTO
    • Use an update DTO without memberId to avoid Jackson deserializing it into the entity.
  • Audit logging
    • Record address modification operations and operator details for later audit and anomaly tracing.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions