The POST was guarded, the GET on the same URL was not: cross-tenant PII disclosure in CoopCycle (GET /api/stores/{id}/addresses)
TL;DR - What: In coopcycle-web - the open-source logistics and marketplace platform that worker-owned courier co-operatives self-host instead of renting a commercial delivery app - the operationGET /api/stores/{id}/addresses was declared with nosecurity expression at all. Every sibling operation on the same resource family had one. So did thePOST to that exact same URL. The provider behind theGET filtered on the path{id} and nothing else. - Impact: Any authenticated account - and self-registration is open - could walk {id} from 1 upward and read every store's delivery-address book: recipientcontactName ,streetAddress ,postalCode . On a shared instance hosting many co-ops, that is a platform-wide cross-tenant leak of the home addresses of people who ordered dinner. CWE-862 + CWE-639. I score it CVSS v3.1 6.5 (Medium) -AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:N/A:N . That is my score: there is no advisory and no vendor rating, and the load-bearing metric isC:H for platform-wide recipient PII. - Fixed: commit a65d9f9e , two days after I reported it, tagged as v5.6.0 five minutes later. Reported by me, Santosh Kumar Puppala, under coordinated disclosure. No advisory was published - this was a silent fix - and a CVE has been requested and is pending. Why you should care CoopCycle is not a SaaS company. It is a licence and a shared codebase, available only to worker-owned businesses, and a single deployment routinely carries many stores belonging to unrelated operators - the platform models Store as a first-class tenant precisely because that is how it is used. Multi-tenancy here is not an enterprise feature bolted on for a big customer. It is the shape of the product. And the data behind this particular endpoint is about as personal as delivery software gets. A store's address book is not company data. It is a list of customers' homes - name, contact name, street, postcode - accumulated across every delivery that store has ever made. The endpoint that returned it required nothing but a login. Registration is open to the public. The setup CoopCycle is Symfony 6 with API Platform 3. In API Platform, authorization on a resource is declarative: you put a security: expression on each operation, written in Symfony's expression language, and the framework evaluates it before the operation runs. If you omit the key, there is no check beyond whatever the firewall did - and CoopCycle's api_platform.yaml sets no global default. The project uses this well. Here is src/Entity/Store.php in v5.5.0 - every sibling sub-resource on /stores/{id}/* carrying its guard: new Get( uriTemplate: '/stores/{id}/time_slots', security: "is_granted('ROLE_DISPATCHER') or is_granted('ROLE_COURIER') or is_granted('edit', object)" ), new Get( uriTemplate: '/stores/{id}/payment_methods', security: "is_granted('ROLE_DISPATCHER') or is_granted('ROLE_COURIER') or is_granted('edit', object)" ), new Post( uriTemplate: '/stores/{id}/addresses', // ownsStore($subject) . The tenant boundary is real, it is enforced, and the developers clearly know where it goes. That is the negative control: this is not a single-tenant app where nobody thought about isolation. The bug The collection read is not declared on Store . It is declared on Address , in a second #[ApiResource] block in src/Entity/Address.php : #[ApiResource( uriTemplate: '/stores/{id}/addresses', types: ['http://schema.org/Place'], operations: [new GetCollection()], // new Link(fromClass: Store::class, fromProperty: 'addresses') ], normalizationContext: ['groups' => ['address']], provider: StoreAddressesProvider::class )] new GetCollection() . No expression, no argument, nothing. And the provider it delegates to takes the store id straight off the path: private function getDropoffAddresses($storeId) { $qb = $this->entityManager->getRepository(Address::class)->createQueryBuilder('a'); $qb->join(Task::class, 't', Join::WITH, 'a.id = t.address'); $qb->join(Delivery::class, 'd', Join::WITH, 'd.id = t.delivery AND d.store = :store'); $qb->andWhere('t.type = :type') ->setParameter('store', $storeId) // setParameter('type', 'DROPOFF'); return $qb->getQuery()->getResult(); } Note that this hand-rolled QueryBuilder also sidesteps any Doctrine ownership extension the project might add later - the ?type=dropoff branch would keep leaking even if someone bolted a global filter onto the ORM. Now, why would a team this careful about security: expressions leave one operation bare? Reading the fix answered it, and the answer is more interesting than "they forgot." The house idiom is is_granted('edit', object) . On a sub-resource collection, there is no object - API Platform has no single entity to hand the voter. This is not my inference about the framework; the codebase says so itself, in a comment I'll come back to in a moment that carries a link to API Platform's own subresources documentation. So the one idiom the whole codebase leans on is precisely the one unavailable on this shape of endpoint. The sibling POST operates on a single store, so is_granted('edit', object) works there and was used. The GET returns a collection, so it doesn't, and nothing was used instead. The endpoint that got no authorization check was the one endpoint where the project's standard authorization check was not available - the gap tracked the framework's limitation, not the developers' attention. That is the shape worth remembering, because it generalises far past PHP. Wherever a framework's ergonomic guard covers 95% of your routes, the missing 5% is not randomly distributed. It is exactly the awkward shape - the sub-resource, the custom action, the bulk endpoint, the streaming response - and that is where you should look first. Proof of concept I confirmed this end to end on a local bring-up of the v5.5.0 tag: the repo's own docker/php image, PostGIS 16, Redis, nginx, schema created with the project's own console commands and seeded with the project's own Alice fixture loader. Three synthetic accounts, all fictional data. - Store A ( id=1 ), owned bypoc_storeA_owner . One address book entry, seeded with a marker:contactName = "Synthetic Recipient A - IDOR-POC-MARKER-2026" ,streetAddress = "42 Rue Secret Store A Only" . - Store B ( id=2 ), owned bypoc_storeB_owner . A different, non-overlapping entry. - poc_plain_customer -ROLE_USER only, no store, no dispatcher or courier role. The self-registered-customer equivalent. [LEAK] GET /api/stores/1/addresses (token = Store B's owner) -> 200 "contactName":"Synthetic Recipient A - IDOR-POC-MARKER-2026", "streetAddress":"42 Rue Secret Store A Only" 200 identical body 200 same single record 403 [CONTROL] POST /api/stores/1/addresses (token = Store B's owner) -> 403 The two 403s are the whole argument. The same token, against the same store, on sibling operations - including a write to the very URL that just leaked on read - is correctly denied. That kills the "couriers and dispatchers legitimately need this data" objection before anyone raises it: the operation was not open to couriers and dispatchers, it was open to everybody with a password, and the project's own adjacent code says what the intended audience was. Everything above ran against a container on 127.0.0.1 . Nothing was ever sent to a live co-op. The fix Commit a65d9f9e , "Fix store addresses security", landed 2026-07-21, two days after my email, and v5.6.0 was tagged five minutes later. Three files changed. The operation declaration: // before operations: [new GetCollection()], // after operations: [ new GetCollection(security: "is_granted('view', request)") ], Not is_granted('view', object) - request . That works because StoreVoter already carried an escape hatch for exactly this problem, with a comment naming the framework issue and linking API Platform's docs: // Needed for /api/stores/{id}/deliveries endpoint // https://api-platform.com/docs/v4.0/core/subresources/#security if (!$subject instanceof Store && !$subject instanceof Request) { return false; } // ... if ($subject instanceof Request) { $subject = $this->entityManager->getRepository(Store::class)->find($subject->get('id')); } The workaround for the sub-resource limitation was already in the codebase, already used by /stores/{id}/deliveries . This endpoint just never got it. The commit also adds a firewall entry so the token resolves on this path, and - the part I'd frame and hang on a wall - it fixes the test suite: # BEFORE - bob owns store "Acme" (id 1), and this scenario asserted 200 on store 2 When the user "bob" sends a "GET" request to "/api/stores/2/addresses?type=dropoff" Then the response status code should be 200 # AFTER - bob reads his own store, and two new scenarios pin the denial When the user "bob" sends a "GET" request to "/api/stores/1/addresses?type=dropoff" Then the response status code should be 200 Scenario: Not authorized to list another store's addresses with JWT When the user "bob" sends a "GET" request to "/api/stores/2/addresses" Then the response status code should be 403 A green Behat suite had been asserting the cross-tenant read as correct behaviour. Honesty note on evidence class: I verified the fix from the public repository - the commit contents, and git tag --contains putting it in v5.6.0 onward. I did not rebuild a patched image and re-run the PoC against it, so this is "confirmed from the commit and the release" rather than a re-tested NOT_REPRODUCED verdict. The weaker of the two, and worth labelling as such. Takeaways Diff the guarded operations against the exposed ones, and start with the odd one out. In a declarative-authorization codebase, the audit is nearly mechanical: list every operation, list every security: expression, subtract. On CoopCycle that diff was one line long. The same query works on DRF permission_classes , on Spring @PreAuthorize , on NestJS guards - anywhere the check is an annotation, because an annotation can be absent and absence renders as nothing at all. Read/write asymmetry on the same URL is the highest-signal t
Comments
No comments yet. Start the discussion.