On main: add-mutations

[jordan-dr]

No description provided.

[dryrunsecurity]

🟡 Please give this pull request extra attention during review.

This pull request introduces multiple high-confidence IDOR vulnerabilities in notification-related GraphQL mutations, where users can potentially access, modify, or delete notifications belonging to other users due to a complete lack of authorization checks in the DeleteNotification and MarkNotificationAsRead mutations.

⚠️ Potential IDOR Vulnerability in app/graphql/mutations/notifications/mark_notification_as_read.rb

Vulnerability

Potential IDOR Vulnerability

Description

This code represents a potential IDOR vulnerability because it lacks user authorization checks when retrieving and modifying a notification. The mutation simply finds a notification by ID without verifying that the current user has the right to access or modify that specific notification. An attacker could potentially manipulate the notification ID to read or modify notifications belonging to other users.

rails-projects/app/graphql/mutations/notifications/mark_notification_as_read.rb

Lines 1 to 26 in 71f3664

	module Mutations
	module Notifications
	class MarkNotificationAsRead < BaseMutation
	graphql_name 'MarkNotificationAsRead'
	# Input argument to indicate which notification to update.
	argument :id, ID, required: true
	# The response includes the updated notification and any errors.
	field :notification, Types::NotificationType, null: true
	field :errors, [String], null: false
	def resolve(id:)
	notification = Notification.find_by(id: id)
	return { notification: nil, errors: ["Notification not found"] } unless notification
	notification.read = true
	if notification.save
	{ notification: notification, errors: [] }
	else
	{ notification: nil, errors: notification.errors.full_messages }
	end
	end
	end
	end
	end

⚠️ Potential IDOR Vulnerability in app/graphql/mutations/notifications/delete_notification.rb

Vulnerability

Potential IDOR Vulnerability

Description

This code represents a potential Insecure Direct Object Reference (IDOR) vulnerability because it allows deletion of a notification by ID without verifying the requester's ownership or authorization. The resolve method simply finds a notification by ID and attempts to destroy it, with no checks to ensure the current user has the right to delete that specific notification. This could allow an attacker to delete notifications belonging to other users by guessing or enumerating notification IDs.

rails-projects/app/graphql/mutations/notifications/delete_notification.rb

Lines 1 to 25 in 71f3664

	module Mutations
	module Notifications
	class DeleteNotification < BaseMutation
	graphql_name 'DeleteNotification'
	# Input argument to indicate which notification to delete.
	argument :id, ID, required: true
	# The response indicates success and returns any errors.
	field :success, Boolean, null: false
	field :errors, [String], null: false
	def resolve(id:)
	notification = Notification.find_by(id: id)
	return { success: false, errors: ["Notification not found"] } unless notification
	if notification.destroy
	{ success: true, errors: [] }
	else
	{ success: false, errors: notification.errors.full_messages }
	end
	end
	end
	end
	end

✨ Code Policies (3)

Policy	graphql-auth-check
Result	The new mutations (DeleteNotification and MarkNotificationAsRead) bypass authentication by not implementing the authorization checks available in BaseMutation. While BaseMutation provides an 'authorize' method for implementing authentication checks, neither mutation uses it, allowing unrestricted access to delete or modify any notification by ID.

Policy

Auth Policy at Acme

Result

The changes do not properly enforce authorization. The mutations allow direct access to notifications by ID without any authorization checks, violating the requirement to use the AllGood authorization library. Neither mutation implements authorization checks through BaseMutation's authorize method or uses AllGood.authorize! as required. This creates potential IDOR vulnerabilities by allowing any user to delete or mark as read any notification without permission verification.

Policy

auth-check

Result

The changes appear to bypass authentication. While the BaseMutation class has an 'authorize' method available, neither DeleteNotification nor MarkNotificationAsRead mutations implement any authentication or authorization checks. The mutations allow direct access to find and modify notifications without verifying the user's authentication status or authorization to access these specific notifications. The base class's authorize method is optional and must be explicitly called, but it is not being used in either mutation.

💭 Unconfirmed Findings (3)

Vulnerability	Potential Injection via Unvalidated Notification ID
Description	In the delete_notification.rb file, the find_by method uses a notification ID parameter without proper sanitization, which could enable SQL injection attacks by allowing malicious manipulation of database queries.

Vulnerability	Potential Authorization Bypass
Description	The delete_notification.rb mutation lacks explicit authorization checks, potentially allowing users to delete notifications that do not belong to them, which represents a significant security risk.

Vulnerability	Potential Unauthorized Notification Access
Description	In the mark_notification_as_read.rb mutation, there are no authentication or authorization checks, enabling users to modify notification states without proper verification of their permissions.

---

All finding details can be found in the DryRun Security Dashboard.