Skip to content

Security: Unsafe dynamic class instantiation from user input + zero RBAC + SQL injection #14

@lighthousekeeper1212

Description

@lighthousekeeper1212

Summary

A security audit identified 7 authorization vulnerabilities (1 Critical, 3 High, 2 Medium, 1 Low) in the Discover ERP system.

Critical Finding

Unsafe Dynamic Class Instantiation from User Input (CRITICAL)

Five controllers/actions construct class names directly from user HTTP input:

PrintController.php:27-30:

\$model = \$request->input('model');
\$modelClass = "\\App\Models\\" . \$model;
\$orders = \$modelClass::query()->findOrFail(\$orderIds);

Same pattern in OrderReview.php:71, OrderDelete.php:31, Delete.php:37, BatchCreateProSave.php:54. The model parameter from user input determines which class is instantiated. PrintController has NO validation at all. An attacker could supply path-traversal-style values to instantiate unintended classes.

HIGH Findings

SQL Injection via DB::raw() (HIGH)

Observers/StockHistoryObserver.php:38,66:

'num' => DB::raw("num + \$stockHistoryModel->in_num"),

Model properties interpolated directly into raw SQL. Values originate from user-editable form fields with \$guarded = ['id'] (all other fields mass-assignable).

Unsafe call_user_func with User-Controlled Method Names (HIGH)

Five classes use call_user_func with method names derived from user input:

  • OrderReview.php:81\$request->input('table') → method name
  • BatchStockSelectSave.php:61request()->input('_table') → method name
  • Statement.php:45\$request->input('_func') → method name

Any existing method on these classes can be invoked.

Zero Role-Based Authorization (HIGH)

Despite Dcat Admin's permission system being enabled, NO controller, action, or form checks any permission. Zero calls to hasPermission(), isAdministrator(), isRole(), or inRoles() anywhere. Any authenticated admin user can perform all ERP operations: financial settlements, order approvals, inventory manipulation, report generation.

Recommended Fixes

  1. Whitelist allowed model names instead of using user input directly:
    \$allowed = ['PurchaseInOrder', 'SaleOutOrder', ...];
    abort_unless(in_array(\$model, \$allowed), 400);
  2. Parameterize DB::raw(): use DB::raw("num + ?") with bindings
  3. Use switch/whitelist for dynamic method dispatch instead of call_user_func
  4. Implement role-based authorization — assign permissions per role in Dcat Admin
  5. Define \$fillable on models instead of only guarding id

Disclosure

Filed in good faith. No exploit code provided.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions