Create tickets from policies vulnerabilities

[edimossilva]

CRITICAL: HospitalPolicy - Missing Ownership Verification

Severity: CRITICAL

Vulnerability Type

Insecure Direct Object Reference (IDOR) - Missing Authorization Check

---

Summary

The HospitalPolicy allows ANY authenticated user to update or delete ANY hospital in the system. Since hospitals are global resources (not user-scoped), this vulnerability allows any user to modify or delete hospitals used by other users, potentially corrupting medical records and breaking data integrity.

---

Affected Files

File	Line(s)	Issue
app/policies/hospital_policy.rb	12-17	update? and destroy? only check user.present?
app/controllers/api/v1/hospitals_controller.rb	27, 38	Calls authorize(hospital) with weak policy
app/operations/hospitals/find.rb	10	Uses Hospital.find(id) without user scoping
app/models/hospital.rb	all	No user association - hospitals are global

---

Vulnerable Code

app/policies/hospital_policy.rb

class HospitalPolicy < ApplicationPolicy
  def index?
    user.present?
  end

  def create?
    user.present?
  end

  def update?
    user.present?  # VULNERABLE: Only checks if user is logged in
  end

  def destroy?
    user.present?  # VULNERABLE: Only checks if user is logged in
  end
end

app/controllers/api/v1/hospitals_controller.rb

def update
  authorize(hospital)  # Calls HospitalPolicy#update? which returns true for any user
  result = Hospitals::Update.result(id: hospital.id.to_s, attributes: hospital_params)
  # ...
end

def destroy
  authorize(hospital)  # Calls HospitalPolicy#destroy? which returns true for any user
  result = Hospitals::Destroy.result(id: hospital.id.to_s)
  # ...
end

private

def hospital
  @hospital ||= Hospitals::Find.result(id: params[:id]).hospital  # Fetches ANY hospital by ID
end

---

Exploitation Steps

Prerequisites

• Valid authenticated API token for any user account

Attack Scenario 1: Update Any Hospital

# Step 1: List hospitals to get IDs (or enumerate from 1)
curl -X GET "https://api.example.com/api/v1/hospitals" \
  -H "Authorization: Bearer <attacker_token>"

# Response shows hospitals with IDs
# { "hospitals": [{ "id": 1, "name": "Hospital A" }, { "id": 2, "name": "Hospital B" }] }

# Step 2: Modify another user's hospital data
curl -X PATCH "https://api.example.com/api/v1/hospitals/1" \
  -H "Authorization: Bearer <attacker_token>" \
  -H "Content-Type: application/json" \
  -d '{"name": "HACKED", "address": "Modified Address"}'

# Response: 200 OK - Hospital successfully modified

Attack Scenario 2: Delete Any Hospital

# Delete a hospital used by other users
curl -X DELETE "https://api.example.com/api/v1/hospitals/1" \
  -H "Authorization: Bearer <attacker_token>"

# Response: 200 OK - Hospital deleted
# Impact: All event_procedures referencing this hospital are affected

Attack Scenario 3: ID Enumeration

# Enumerate valid hospital IDs
for id in {1..1000}; do
  response=$(curl -s -o /dev/null -w "%{http_code}" \
    -X DELETE "https://api.example.com/api/v1/hospitals/$id" \
    -H "Authorization: Bearer <attacker_token>")
  if [ "$response" == "200" ]; then
    echo "Hospital $id deleted"
  fi
done

---

Impact Assessment

Data Integrity

• Event Procedures Corruption: Modifying hospital names/addresses affects all related event_procedures
• Soft Delete Cascade: Deleting a hospital (via acts_as_paranoid) may affect related records

Business Impact

• Medical records may become inconsistent
• Audit trails could be compromised
• User trust violation

Compliance Impact

• HIPAA violations (if applicable) - unauthorized modification of medical records
• GDPR data integrity requirements

---

Remediation Options

Option 1: Add User Ownership to Hospital Model (Recommended if hospitals should be user-scoped)

# Migration
class AddUserToHospitals < ActiveRecord::Migration[7.1]
  def change
    add_reference :hospitals, :user, foreign_key: true
  end
end

# Model
class Hospital < ApplicationRecord
  belongs_to :user
  # ...
end

# Policy
class HospitalPolicy < ApplicationPolicy
  def update?
    user_owner?
  end

  def destroy?
    user_owner?
  end
end

Option 2: Admin-Only Modification (If hospitals are intentionally global)

class HospitalPolicy < ApplicationPolicy
  def update?
    user.admin?
  end

  def destroy?
    user.admin?
  end
end

Option 3: Reference-Based Protection (Prevent deletion if in use)

class HospitalPolicy < ApplicationPolicy
  def destroy?
    user.admin? && record.event_procedures.empty?
  end
end

---

Testing Recommendations

Add the following tests to spec/policies/hospital_policy_spec.rb:

RSpec.describe HospitalPolicy do
  describe '#update?' do
    context 'when user does not own the hospital' do
      it 'returns false' do
        user = create(:user)
        hospital = create(:hospital, user: create(:another_user))

        policy = described_class.new(user, hospital)

        expect(policy.update?).to be false
      end
    end
  end

  describe '#destroy?' do
    context 'when user does not own the hospital' do
      it 'returns false' do
        user = create(:user)
        hospital = create(:hospital, user: create(:another_user))

        policy = described_class.new(user, hospital)

        expect(policy.destroy?).to be false
      end
    end
  end
end

---

References

• OWASP IDOR: https://owasp.org/www-project-web-security-testing-guide/latest/4-Web_Application_Security_Testing/05-Authorization_Testing/04-Testing_for_Insecure_Direct_Object_References
• CWE-639: Authorization Bypass Through User-Controlled Key