create tckets for DashboardAmountByDay

[edimossilva]

MEDIUM: DashboardAmountByDay - Admin Data Exposure

Severity: MEDIUM

Vulnerability Type

Insecure Direct Object Reference (IDOR) via User ID Parameter / Information Disclosure

---

Summary

The EventProcedures::DashboardAmountByDay service accepts a user_id parameter that allows fetching event procedure data for any user. When user_id is not provided, it returns data for ALL users in the system. While the endpoint is restricted to admin users, this behavior should be explicitly documented and may still represent a risk if admin accounts are compromised.

---

Affected Files

File	Line(s)	Issue
app/controllers/api/v1/event_procedures_dashboard_controller.rb	21-22	Accepts user_id from params
app/operations/event_procedures/dashboard_amount_by_day.rb	14-15	Queries all users' data if user_id is nil
app/policies/event_procedure_dashboard_policy.rb	4-6	Only checks user.admin?

---

Vulnerable Code Analysis

Controller

# app/controllers/api/v1/event_procedures_dashboard_controller.rb
def amount_by_day
  authorize :amount_by_day, policy_class: EventProcedureDashboardPolicy

  result = EventProcedures::DashboardAmountByDay.result(amount_by_day_params)
  # ...
end

private

def amount_by_day_params
  params.permit(:start_date, :end_date, :user_id)  # user_id is user-controlled
end

Service Operation

# app/operations/event_procedures/dashboard_amount_by_day.rb
input :user_id, allow_nil: true, default: -> {}

def call
  set_date_times
  event_procedures = EventProcedure.date_between(start_date: start_date_time, end_date: end_date_time)

  # ISSUE: If user_id is nil, returns ALL event procedures from ALL users
  event_procedures = event_procedures.where(user_id:) if user_id.present?

  # ...
end

Policy

# app/policies/event_procedure_dashboard_policy.rb
class EventProcedureDashboardPolicy < ApplicationPolicy
  def amount_by_day?
    user.admin?  # Only admins can access, but they can see ALL data
  end
end

---

Risk Assessment

Mitigating Factors

1. Endpoint is restricted to admin users only
2. Admin users may legitimately need to view all users' data for reporting

Risk Factors

1. Admin Account Compromise: If an admin account is compromised, attacker gets access to all users' medical procedure data
2. Lack of Audit Trail: No logging of which user's data is being accessed
3. Implicit Behavior: Default behavior exposes all data when user_id is omitted

---

Exploitation (Requires Admin Access)

# As admin: Get all users' event procedures (default behavior)
curl -X GET "http://localhost:3000/api/v1/event_procedures_dashboard/amount_by_day?start_date=01/01/2024&end_date=31/12/2024" \
  -H "Authorization: Bearer <admin_token>"

# Response includes aggregated data from ALL users

# As admin: Get specific user's event procedures
curl -X GET "http://localhost:3000/api/v1/event_procedures_dashboard/amount_by_day?start_date=01/01/2024&end_date=31/12/2024&user_id=123" \
  -H "Authorization: Bearer <admin_token>"

# Response includes data only for user 123

---

Security Concerns

1. No User ID Validation

The user_id parameter is not validated to ensure it's a valid user ID:

event_procedures = event_procedures.where(user_id:) if user_id.present?

An admin could provide any arbitrary ID, and if it doesn't match any user, they'd just get an empty result set. This is not exploitable but is poor practice.

2. Missing Audit Logging

There's no logging of admin access to other users' data, making it difficult to detect unauthorized access patterns.

3. Implicit "All Users" Mode

When user_id is not provided, the service returns all users' data. This should be an explicit parameter choice, not a default.

---

Recommendations

Option 1: Require Explicit User ID (Recommended)

# app/operations/event_procedures/dashboard_amount_by_day.rb
input :user_id, type: String  # Make user_id required

def call
  set_date_times
  event_procedures = EventProcedure
    .date_between(start_date: start_date_time, end_date: end_date_time)
    .where(user_id:)
  # ...
end

Option 2: Add Explicit "all_users" Parameter

input :user_id, allow_nil: true
input :include_all_users, type: [TrueClass, FalseClass], default: false

def call
  set_date_times
  event_procedures = EventProcedure.date_between(...)

  if include_all_users
    # Explicitly requested all users - log this action
    Rails.logger.info("Admin #{current_user.id} requested all users' dashboard data")
  else
    fail!(error: "user_id is required") if user_id.blank?
    event_procedures = event_procedures.where(user_id:)
  end
  # ...
end

Option 3: Add Audit Logging

# app/controllers/api/v1/event_procedures_dashboard_controller.rb
def amount_by_day
  authorize :amount_by_day, policy_class: EventProcedureDashboardPolicy

  # Audit log for admin access
  if params[:user_id].present?
    Rails.logger.info("[AUDIT] Admin #{current_user.id} accessed dashboard for user #{params[:user_id]}")
  else
    Rails.logger.warn("[AUDIT] Admin #{current_user.id} accessed ALL USERS dashboard data")
  end

  result = EventProcedures::DashboardAmountByDay.result(amount_by_day_params)
  # ...
end

---

Design Decision Required

This vulnerability may be by design for admin reporting purposes. The development team should clarify:

1. Should admins be able to view all users' data in aggregate?
2. Should individual user data access require a specific user_id?
3. Is audit logging required for compliance (HIPAA, GDPR)?

If this is intentional admin functionality, document it as such and implement audit logging.

---

References

• OWASP Access Control: https://owasp.org/www-project-web-security-testing-guide/latest/4-Web_Application_Security_Testing/05-Authorization_Testing/
• CWE-285: Improper Authorization