fix: add role and ownership guards to all order-lifecycle handlers#558
Open
Ridanshi wants to merge 1 commit into
Open
fix: add role and ownership guards to all order-lifecycle handlers#558Ridanshi wants to merge 1 commit into
Ridanshi wants to merge 1 commit into
Conversation
Five globally accessible order-lifecycle functions had no authorization checks. Any authenticated session — regardless of role or account — could call them from the browser console to trigger unauthorized state changes and reward issuance. confirmPlantReceipt (primary issue Shruti070107#541) No role check and no plant-ownership check meant any user could: - call confirmPlantReceipt(orderId) on any order in at_plant state - complete the order and mint $RGX reward tokens for the provider - do so repeatedly to inflate balances Two guards are now enforced before any other processing: if (SESSION.role !== 'plant') → reject if (o.plantId !== SESSION.id) → reject Also incorporated alongside: the confirm() dialog is moved before all mutations so cancellation is a guaranteed no-op; earnedTokens is declared as let outside the if(providerAcc) block to fix a ReferenceError that fired when the provider account was absent. riderAccept No role check and no order-status guard allowed any user to overwrite any order — including already-completed ones — setting status to 'assigned' and claiming the riderId. Guards added: if (SESSION.role !== 'rider') → reject if (o.status !== 'requested') → reject (order unavailable) riderUpdate No role check and no rider-ownership check allowed any authenticated user to advance any order's delivery status. Guards added: if (SESSION.role !== 'rider') → reject if (o.riderId !== SESSION.id) → reject confirmPickup Same rider-ownership gap as riderUpdate, plus a missing null check on the order object. Guards added: if (!o) → reject (null safety) if (SESSION.role !== 'rider') → reject if (o.riderId !== SESSION.id) → reject cancelOrder No role check and no ownership check allowed any user to cancel any dispatch. Guards added: if (SESSION.role !== 'provider') → reject if (o.providerId !== SESSION.id) → reject Added scripts/test-order-authorization.mjs (39 assertions) covering all five handlers: role blocked, ownership blocked, correct-session succeeds, rewards issued only after authorization, cancellation leaves no side effects, and a regression test demonstrating that a plant account cannot complete an order assigned to a different plant. Closes Shruti070107#541
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #541
Issue confirmed
Five globally accessible order-lifecycle functions had no authorization checks. Any authenticated session — regardless of role — could call them directly from the browser console to trigger unauthorized state changes and reward issuance.
Handlers fixed
confirmPlantReceipt(primary issue)window.confirmPlantReceiptis a global function with no role or ownership check. Any user could callconfirmPlantReceipt('any-order-id')to complete any order inat_plantstate and mint$RGXreward tokens for the provider's account.Two guards are now the first checks after the idempotency guard — before any processing, any mutation, any reward calculation:
The
confirm()dialog is also moved before all mutations (so cancellation is a zero-side-effect no-op), andearnedTokensis declared asletoutside theif (providerAcc)block to fix aReferenceErrorthat fired when the provider account was absent.riderAcceptNo role check and no order-status guard allowed any user to overwrite any order — including completed ones — claiming the
riderId. Added:SESSION.role !== 'rider'guardo.status !== 'requested'guard (prevents reassigning already-in-flight orders)riderUpdateNo role or rider-ownership check. Added:
SESSION.role !== 'rider'guardo.riderId !== SESSION.idguardconfirmPickupSame rider-ownership gap as
riderUpdate, plus a missing null check on the order object. Added null safety and both guards.cancelOrderNo role or ownership check. Any user could cancel any dispatch. Added:
SESSION.role !== 'provider'guardo.providerId !== SESSION.idguardTest results
Test 20 (regression) directly proves the exploit: a plant session for
plant-attackercannot complete an order assigned toplant-correct, no tokens are issued, and the provider's balance is unchanged.