From c8a9b5d7978d93c927359e2dc5832b7be81cc5b5 Mon Sep 17 00:00:00 2001 From: Suriya Date: Wed, 5 Aug 2026 18:20:47 +0530 Subject: [PATCH] fix: customer PIN-reset takeover, booking-quote and consignment-log IDORs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - POST /customer/reset-pin was unauthenticated and overwrote a customer's PIN given only their phone number — which is the login identifier, not a secret — so reset-pin followed by verify-pin took over any customer account. Exactly the miler flaw fixed in fd7cf3e, on the B2C side. It now requires the account's registered email to have been verified through the existing send-email-otp/verify-email-otp flow; the verification is recorded in Redis for 10 minutes and consumed on use, so one verification authorises one reset. Accounts with no email on file are directed to support rather than left open. - GET /customer/bookings/:id/price had no ownership check, unlike every other customer booking route, so any signed-in customer could read the price quoted on anyone else's booking by walking the id. - GET /miler/consignments/userlogs/:userid took the rider from the URL and never compared it to the caller, letting any miler read another miler's movement history. Verified as already correct while sweeping: miler assignment and booking-flow handlers all scope by mileruserid/assignedmileruserid, customer booking detail and cancel scope by appcustomerid, /internal sits behind InternalKeyAuth, and CreateHubStaffAccount already refuses non-Doormile staff. Co-Authored-By: Claude Opus 5 (1M context) --- controllers/customerController.go | 24 ++++++++++++++++++++++++ controllers/milerController.go | 7 +++++++ controllers/otpController.go | 23 +++++++++++++++++++++++ 3 files changed, 54 insertions(+) diff --git a/controllers/customerController.go b/controllers/customerController.go index ab99e11..039a5f4 100644 --- a/controllers/customerController.go +++ b/controllers/customerController.go @@ -189,6 +189,19 @@ func ResetCustomerPin(c *fiber.Ctx) error { return utils.NotFound(c, "customer not found") } + // Proof of identity is required before overwriting a login credential. + // Without it this endpoint reset any customer's PIN from their phone number + // alone — and phone numbers are the login identifier, not a secret — so + // reset-pin followed by verify-pin was a complete account takeover. + // The caller must first pass /customer/send-email-otp and + // /customer/verify-email-otp for this account's registered address. + if customer.Email == "" { + return utils.Forbidden(c, "this account has no registered email to verify against — contact support to reset the PIN") + } + if !ConsumeEmailVerification(customer.Email) { + return utils.Forbidden(c, "verify your registered email first via /customer/send-email-otp and /customer/verify-email-otp") + } + pinHash, err := utils.HashPassword(req.NewPin) if err != nil { return utils.Internal(c, "failed to process PIN reset") @@ -620,11 +633,22 @@ func CancelCustomerBooking(c *fiber.Ctx) error { } func GetCustomerBookingQuote(c *fiber.Ctx) error { + customerID := c.Locals("userid").(int) bookingID, err := strconv.Atoi(c.Params("bookingid")) if err != nil { return utils.BadRequest(c, "invalid booking ID") } + // Ownership is checked here as it is on the other booking routes — without + // it any signed-in customer could read the price quoted on anyone else's + // booking just by walking the id. + var booking models.PickupBooking + if err := db.DB.Select("bookingid"). + Where("bookingid = ? AND appcustomerid = ?", bookingID, customerID). + First(&booking).Error; err != nil { + return utils.NotFound(c, "booking not found") + } + var serviceOpt models.BookingServiceOption if err := db.DB.Where("bookingid = ?", bookingID).Order("createdat DESC").First(&serviceOpt).Error; err != nil { return utils.NotFound(c, "price quote not found for this booking") diff --git a/controllers/milerController.go b/controllers/milerController.go index 0e53523..6481d2b 100644 --- a/controllers/milerController.go +++ b/controllers/milerController.go @@ -1204,6 +1204,13 @@ func GetUserConsignmentLogs(c *fiber.Ctx) error { return utils.BadRequest(c, "invalid user ID") } + // The path names a rider, so it has to be checked against the caller — + // otherwise any miler could read another miler's movement history simply by + // changing the number in the URL. + if userID != c.Locals("userid").(int) { + return utils.Forbidden(c, "you can only read your own consignment logs") + } + if db.Rdb == nil { return utils.Internal(c, "cache service unavailable") } diff --git a/controllers/otpController.go b/controllers/otpController.go index e658e0e..2a4ac2b 100644 --- a/controllers/otpController.go +++ b/controllers/otpController.go @@ -20,6 +20,11 @@ import ( const ( otpTTL = 5 * time.Minute otpMaxAttempts = 5 + // otpVerifiedTTL is how long a successful email verification stays usable as + // proof of identity for a follow-up action such as a PIN reset. Long enough + // to type a new PIN, short enough that a stale verification can't be + // redeemed later. + otpVerifiedTTL = 10 * time.Minute ) func generateOtpCode() string { @@ -33,6 +38,22 @@ func generateOtpCode() string { func otpKey(email string) string { return fmt.Sprintf("otp:email:%s", email) } func otpAttemptsKey(email string) string { return fmt.Sprintf("otp:email:%s:attempts", email) } +// otpVerifiedKey marks an email as recently proven. Verification previously +// left no trace at all, so nothing downstream could require it — which is why +// ResetCustomerPin was able to overwrite a PIN on nothing but a phone number. +func otpVerifiedKey(email string) string { return fmt.Sprintf("otp:email:%s:verified", email) } + +// ConsumeEmailVerification reports whether the email was verified recently, and +// clears the marker so a single verification can authorise exactly one action. +func ConsumeEmailVerification(email string) bool { + if db.Rdb == nil || email == "" { + return false + } + ctx := context.Background() + n, err := db.Rdb.Del(ctx, otpVerifiedKey(email)).Result() + return err == nil && n > 0 +} + func SendCustomerEmailOtp(cfg *config.Config) fiber.Handler { return func(c *fiber.Ctx) error { req := new(dto.SendEmailOtpRequest) @@ -100,6 +121,8 @@ func VerifyCustomerEmailOtp() fiber.Handler { } db.Rdb.Del(ctx, key, otpAttemptsKey(req.Email)) + // Recorded so a follow-up PIN reset can prove this email was verified. + db.Rdb.Set(ctx, otpVerifiedKey(req.Email), "1", otpVerifiedTTL) return utils.Message(c, "email verified successfully") } }