Skip to content

Security: Employee Self-Approval via Mass Assignment + Leave IDOR + Weak Password Generation #76

@lighthousekeeper1212

Description

@lighthousekeeper1212

Summary

During a security review of the HRM system, I identified 7 authorization vulnerabilities related to mass assignment, IDOR, and weak credentials.

Findings

1. CRITICAL: Employee Self-Approval of Leave via Mass Assignment

File: Employee/Leaves/LeavesController.php:150 + EmployeeLeave model

The employee-side LeavesController::update() passes $request->all() to the repository's update() method. The EmployeeLeave model uses $guarded = ['id'], meaning approved is mass-assignable. An employee can inject approved=1 into the POST data when updating a pending leave to self-approve without admin review.

2. HIGH: Leave Show IDOR (1-of-N)

File: Employee/Leaves/LeavesController.php:115

The show() method fetches a leave record by ID without calling checkValidity(). The edit(), update(), and destroy() methods all call checkValidity($employeeLeave->user_id), but show() does not. Any employee can view any other employee's leave details.

3. HIGH: Weak Password Generation

File: Pim/EmployeesController.php:132

Employee passwords are generated using rand(), which is not cryptographically secure and produces a limited keyspace of numeric-only values (~31 bits). The password is emailed in plaintext.

4. HIGH: Path Traversal via Storage Route

File: routes/web.php:19-21

The storage route uses (.*) regex and concatenates user input directly into storage_path(). While admin-only, this allows directory traversal.

5. HIGH: Admin Leave Update Mass Assignment

File: Leave/EmployeeLeaveController.php:131-144

Admin leave update uses $request->all() with $guarded=['id'] model, allowing injection of any field.

6. MEDIUM: User Model Mass Assignment

File: User.php:31

The role and password fields are in $fillable, creating a defense-in-depth risk.

7. MEDIUM: Leave Balance Validation Disabled

File: Employee/Leaves/LeaveRequest.php:54

Leave balance validation is commented out (//skipping this for now), allowing unlimited leave requests.

Root Cause

The systemic pattern is $request->all() combined with $guarded = ['id'] models. Every model except User guards only the primary key, making all other fields mass-assignable.

Recommended Fixes

  1. Use $request->only([...specific fields...]) instead of $request->all()
  2. Define explicit $fillable arrays on all models
  3. Add checkValidity() to show() methods
  4. Use random_bytes() for password generation
  5. Sanitize file paths in the storage route
  6. Remove role and password from User $fillable

This was found during a systematic security review.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions