Fix: Forgeable discount coupons: unsigned z85-encoded coupon codes accepted by PUT /rest/basket/:id/coupon/:coupon allow arbitrary discounts up to 99%

frontend/src/app/payment/payment.component.html

@@ -54,7 +54,7 @@
           [innerHtml]="'FOLLOW_FOR_MONTHLY_COUPONS' | translate:{blueSky: blueSkyUrl, reddit: redditUrl}">
         </mat-hint>
         <input #coupon id="coupon" [formControl]="couponControl" matInput type="text" placeholder="{{ 'ENTER_COUPON_CODE' | translate}}">
-        <mat-hint align="end">{{coupon.value?.length || 0}}/10</mat-hint>
+        <mat-hint align="end">{{coupon.value?.length || 0}}/{{couponCodeMaxLength}}</mat-hint>
         @if (couponControl.invalid && (couponControl.errors.minlength || couponControl.errors.maxlength)) {
         <mat-error>
           {{'COUPON_CODE_HINT' | translate}}

frontend/src/app/payment/payment.component.ts

@@ -71,7 +71,8 @@ export class PaymentComponent implements OnInit {
   public redditUrl = null
   public applicationName = 'OWASP Juice Shop'
   private campaignCoupon: string
-  public couponControl: UntypedFormControl = new UntypedFormControl('', [Validators.required, Validators.minLength(10), Validators.maxLength(10)])
+  public readonly couponCodeMaxLength = 35
+  public couponControl: UntypedFormControl = new UntypedFormControl('', [Validators.required, Validators.minLength(10), Validators.maxLength(this.couponCodeMaxLength)])
   public clientDate: any
   public paymentId: any = undefined
   public couponPanelExpanded = false

frontend/src/assets/i18n/en.json

@@ -352,7 +352,7 @@
   "BONUS_POINTS_EARNED": "Bonus Points Earned: {{bonus}}",
   "BONUS_FOR_FUTURE_PURCHASES": "The bonus points from this order will be <em>added 1:1 to your wallet ¤-fund</em> for future purchases!",
   "ENTER_COUPON_CODE": "Please enter your coupon code",
-  "COUPON_CODE_HINT": "Coupon code must be 10 characters long.",
+  "COUPON_CODE_HINT": "Coupon code must be between 10 and 35 characters long.",
   "CHARGED_WALLET": "Wallet successfully charged.",
   "BTN_SHOW_ONLY_TUTORIALS": "Show tutorials only",
   "INFO_FULL_CHALLENGE_MODE": "Complete the remaining tutorial challenges to unveil all {{num}} challenges and unlock the advanced Score Board filters!",

lib/insecurity.ts

@@ -96,9 +96,28 @@ export const userEmailFrom = ({ headers }: any) => {
   return headers ? headers['x-user-email'] : undefined
 }
 
+const couponSigningKey = process.env.COUPON_SIGNING_KEY ?? privateKey
+// signature = one hex nonce digit + truncated HMAC-SHA256(payload + nonce)
+const COUPON_SIGNATURE_LENGTH = 19 // (payload.length + 1 + 19) % 4 === 0, as required by z85
+const COUPON_NONCES = '0123456789abcdef'
+// z85 output may contain '%' (breaks URL path decoding) or '{...}' (interpreted as key sequence by automated typing)
+const UNSAFE_COUPON_CHARS = /%|\{[^{}]*\}/
+
+const couponSignature = (payload: string, nonce: string) => {
+  const digest = crypto.createHmac('sha256', couponSigningKey).update(payload + nonce).digest('hex')
+  return nonce + digest.substring(0, COUPON_SIGNATURE_LENGTH - 1)
+}
+
 export const generateCoupon = (discount: number, date = new Date()) => {
-  const coupon = utils.toMMMYY(date) + '-' + discount
-  return z85.encode(coupon)
+  const payload = utils.toMMMYY(date) + '-' + discount
+  let coupon = ''
+  for (const nonce of COUPON_NONCES) {
+    coupon = z85.encode(payload + '-' + couponSignature(payload, nonce))
+    if (!UNSAFE_COUPON_CHARS.test(coupon)) {
+      break
+    }
+  }
+  return coupon
 }
 
 export const discountFromCoupon = (coupon?: string) => {
@@ -107,17 +126,19 @@ export const discountFromCoupon = (coupon?: string) => {
   }
   const decoded = z85.decode(coupon)
   if (decoded && (hasValidFormat(decoded.toString()) != null)) {
-    const parts = decoded.toString().split('-')
-    const validity = parts[0]
+    const [validity, discount, signature] = decoded.toString().split('-')
+    const expected = couponSignature(validity + '-' + discount, signature.charAt(0))
+    if (signature.length !== expected.length || !crypto.timingSafeEqual(Buffer.from(signature), Buffer.from(expected))) {
+      return undefined
+    }
     if (utils.toMMMYY(new Date()) === validity) {
-      const discount = parts[1]
       return parseInt(discount)
     }
   }
 }
 
 function hasValidFormat (coupon: string) {
-  return coupon.match(/(JAN|FEB|MAR|APR|MAY|JUN|JUL|AUG|SEP|OCT|NOV|DEC)[0-9]{2}-[0-9]{2}/)
+  return coupon.match(/^(JAN|FEB|MAR|APR|MAY|JUN|JUL|AUG|SEP|OCT|NOV|DEC)[0-9]{2}-[0-9]{2}-[0-9a-f]+$/)
 }
 
 // vuln-code-snippet start redirectCryptoCurrencyChallenge redirectChallenge

routes/coupon.ts

@@ -11,7 +11,7 @@ export function applyCoupon () {
   return async ({ params }: Request, res: Response, next: NextFunction) => {
     try {
       const id = params.id
-      let coupon: string | undefined | null = params.coupon ? decodeURIComponent(params.coupon) : undefined
+      let coupon: string | undefined | null = params.coupon ? safeDecodeURIComponent(params.coupon) : undefined
       const discount = security.discountFromCoupon(coupon)
       coupon = discount ? coupon : null
 
@@ -32,3 +32,11 @@ export function applyCoupon () {
     }
   }
 }
+
+function safeDecodeURIComponent (value: string) {
+  try {
+    return decodeURIComponent(value)
+  } catch {
+    return value
+  }
+}

test/server/insecuritySpec.ts

@@ -36,8 +36,28 @@ describe('insecurity', () => {
   describe('generateCoupon', () => {
     it('returns base85-encoded month, year and discount as coupon code', () => {
       const coupon = security.generateCoupon(20, new Date('1980-01-02'))
-      expect(coupon).to.equal('n<MiifFb4l')
-      expect(z85.decode(coupon).toString()).to.equal('JAN80-20')
+      expect(z85.decode(coupon).toString()).to.match(/^JAN80-20-[0-9a-f]+$/)
+    })
+
+    it('signs coupon code so it cannot be forged from a plain encoded payload', () => {
+      const validity = z85.decode(security.generateCoupon(20)).toString().split('-')[0]
+      expect(security.discountFromCoupon(z85.encode(validity + '-99-abc'))).to.equal(undefined)
+      expect(security.discountFromCoupon(z85.encode(validity + '-99-00000000'))).to.equal(undefined)
+      const tampered = z85.decode(security.generateCoupon(10)).toString().replace('-10-', '-99-')
+      expect(security.discountFromCoupon(z85.encode(tampered))).to.equal(undefined)
+      const [validity2, discount, signature] = z85.decode(security.generateCoupon(10)).toString().split('-')
+      const swappedNonce = (parseInt(signature.charAt(0), 16) + 1) % 16
+      expect(security.discountFromCoupon(z85.encode(validity2 + '-' + discount + '-' + swappedNonce.toString(16) + signature.substring(1)))).to.equal(undefined)
+    })
+
+    it('generates coupon codes without characters that break URL decoding or automated input', () => {
+      for (let month = 0; month < 12; month++) {
+        for (const discount of [10, 15, 20, 50, 80, 90, 99]) {
+          const coupon = security.generateCoupon(discount, new Date(2024, month, 1))
+          expect(coupon.length).to.equal(35)
+          expect(coupon).to.not.match(/\{[^{}]*\}/)
+        }
+      }
     })
 
     it('uses current month and year if not specified', () => {