Project

General

Profile

Actions

Bug #4985

closed

Improper handling of "too short" voucher codes (1-2 chars)

Added by Jim Pingle over 8 years ago. Updated over 8 years ago.

Status:
Resolved
Priority:
Very High
Assignee:
Category:
Captive Portal
Target version:
Start date:
08/18/2015
Due date:
% Done:

100%

Estimated time:
Plus Target Version:
Release Notes:
Affected Version:
2.2.x
Affected Architecture:
All

Description

When a user supplies an invalid voucher that is too short (1-2 chars) the system does not properly return an error. Access is not granted, but it can lead to misleading test results and logging/display of the invalid code in online status.

  • The "Test Voucher" page returns a green/success message for any 1-2 char voucher that uses chars from the allowed list set on the portal page, but it claims access was granted for 0 minutes.
  • Attempting to login with a short voucher does not generate an error and it does not grant access
  • Placing a valid voucher in the voucher field after an invalid short voucher will report success (the second voucher was valid) but the portal status and auth log show the invalid voucher as the one granting access along with the minutes from the valid voucher. Example input: "is yWs5HYQqxKS"

Not a security issue as no access was granted. The portal auth code would deny access since "0 minutes" is not valid.

A fix has been committed ( b08758c3f0c446ff2b2b5ab521a32e4a1efe4273 and d0236c7e88e2a874d19269a9a890fbca24607042 ) -- adding this for reference and feedback.

Actions #1

Updated by Luiz Souza over 8 years ago

  • % Done changed from 0 to 100

Two subsequent commits were done to ensure that we ignore vouchers shorter than 5 characters (too short to be a valid voucher):
https://github.com/pfsense/pfsense/commit/9fcbc9ffe38e39e59d7ad098e4e9ba7a378162d9 and https://github.com/pfsense/pfsense/commit/4ccf7dd97ac1e4684fc2c642a1c74209f2bab64a

Actions #2

Updated by Chris Buechler over 8 years ago

  • Status changed from Feedback to Resolved

looks good to me, tests out fine. Jim P confirmed as well

Actions

Also available in: Atom PDF