On main: notification-mutations by jordan-dr · Pull Request #89 · DryRunSecurity/rails-projects

🟡 Please give this pull request extra attention during review.
This pull request contains two potential IDOR (Insecure Direct Object Reference) vulnerabilities in notification-related mutations, where users could potentially mark notifications as read or create notifications for other users without proper authorization checks.
🟡 Potential IDOR Vulnerability in app/graphql/mutations/notifications/update_notification.rb
| Vulnerability |
Potential IDOR Vulnerability |
| Description |
This is a potential IDOR vulnerability because the mutation allows marking a notification as read using only its ID without verifying the current user's ownership or access rights. The code does not implement any user authorization check before performing the action, which means a malicious user could potentially mark notifications belonging to other users as read by guessing or enumerating notification IDs. |
|
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/create_notification.rb
| Vulnerability |
Potential IDOR Vulnerability |
| Description |
This is a true IDOR (Insecure Direct Object Reference) vulnerability. The mutation allows creating a notification for any user_id without performing authorization checks to verify that the current user has the right to create a notification for the specified user. This means a malicious user could potentially create notifications impersonating other users by manipulating the user_id parameter during the GraphQL mutation call. |
|
module Mutations |
|
module Notifications |
|
class CreateNotification < BaseMutation |
|
graphql_name 'CreateNotification' |
|
|
|
# Required arguments for creating a notification. |
|
argument :title, String, required: true |
|
argument :body, String, required: false |
|
argument :user_id, ID, required: true |
|
|
|
# Fields returned by the mutation. |
|
field :notification, Types::NotificationType, null: true |
|
field :errors, [String], null: false |
|
|
|
def resolve(title:, body: nil, user_id:) |
|
notification = Notification.new(title: title, body: body, user_id: user_id) |
|
|
|
if notification.save |
|
{ |
|
notification: notification, |
|
errors: [] |
|
} |
|
else |
|
{ |
|
notification: nil, |
|
errors: notification.errors.full_messages |
|
} |
|
end |
|
end |
|
end |
|
end |
|
end |
All finding details can be found in the DryRun Security Dashboard.