Skip to content

Commit df69de3

Browse files
sethherrclaude
andcommitted
Use Binxtils::Secure.compare? for constant-time token checks
binxtils 0.6.0 ships the helper both ControllerHelpers and BikeServices::ShowCurrentAlerts had their own copy of. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 93936aa commit df69de3

11 files changed

Lines changed: 18 additions & 28 deletions

Gemfile.lock

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ GEM
137137
bcrypt (3.1.22)
138138
benchmark (0.5.0)
139139
bigdecimal (4.1.2)
140-
binxtils (0.5.2)
140+
binxtils (0.6.0)
141141
functionable
142142
loofah
143143
rails
@@ -244,7 +244,7 @@ GEM
244244
dry-logic (~> 1.4)
245245
zeitwerk (~> 2.6)
246246
equalizer (0.0.11)
247-
erb (6.0.6)
247+
erb (6.0.7)
248248
erubi (1.13.1)
249249
excon (0.76.0)
250250
execjs (2.7.0)
@@ -438,7 +438,7 @@ GEM
438438
rdoc (>= 4.0.0)
439439
reline (>= 0.4.2)
440440
jmespath (1.6.2)
441-
json (2.21.1)
441+
json (2.21.2)
442442
jwt (3.2.0)
443443
base64
444444
knapsack_pro (10.0.1)
@@ -674,7 +674,7 @@ GEM
674674
rb-fsevent (0.11.2)
675675
rb-inotify (0.11.1)
676676
ffi (~> 1.0)
677-
rbs (4.1.0)
677+
rbs (4.1.2)
678678
logger
679679
prism (>= 1.6.0)
680680
tsort
@@ -900,7 +900,7 @@ GEM
900900
nokogiri (~> 1.8)
901901
yaml (0.4.0)
902902
yard (0.9.44)
903-
zeitwerk (2.8.2)
903+
zeitwerk (2.8.3)
904904

905905
PLATFORMS
906906
arm64-darwin

app/controllers/api/v1/bikes_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ def create
8080

8181
def authenticate_organization
8282
organization = Organization.friendly_find(params[:organization_slug])
83-
if organization.present? && secure_compare?(params[:access_token], organization.access_token)
83+
if organization.present? && Binxtils::Secure.compare?(params[:access_token], organization.access_token)
8484
@organization = organization
8585
else
8686
render(json: "Not authorized", status: :unauthorized) && return

app/controllers/api/v1/notifications_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ def create
2727
end
2828

2929
def authenticate_notification_permission
30-
unless secure_compare?(params[:access_token], ENV["NOTIFICATIONS_API_KEY"])
30+
unless Binxtils::Secure.compare?(params[:access_token], ENV["NOTIFICATIONS_API_KEY"])
3131
render(json: "Not authorized", status: :unauthorized) && return
3232
end
3333
end

app/controllers/api/v1/organizations_controller.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ def update
1212
@organization = Organization.friendly_find(params[:id])
1313
if @organization.blank?
1414
redirect_to(api_v1_not_found_url) && return
15-
elsif secure_compare?(params[:access_token], @organization.access_token)
15+
elsif Binxtils::Secure.compare?(params[:access_token], @organization.access_token)
1616
if Organization.pos_kinds.include?(params[:manual_pos_kind])
1717
m_kind = (params[:manual_pos_kind] == "no_pos") ? nil : params[:manual_pos_kind]
1818
# We really only want to update orgs when there is a change, otherwise it breaks where
@@ -37,8 +37,8 @@ def verify_organizations_token
3737
redirect_to(api_v1_not_found_url) && return unless @organization.present?
3838

3939
if params[:access_token].present?
40-
return true if secure_compare?(params[:access_token], ENV["ORGANIZATIONS_API_ACCESS_TOKEN"])
41-
return true if secure_compare?(params[:access_token], @organization.access_token)
40+
return true if Binxtils::Secure.compare?(params[:access_token], ENV["ORGANIZATIONS_API_ACCESS_TOKEN"])
41+
return true if Binxtils::Secure.compare?(params[:access_token], @organization.access_token)
4242
end
4343
message = {"401": "Not permitted"}
4444
respond_with(message, status: :unauthorized) && return

app/controllers/bikes/base_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ def assign_bike_stickers(bike_sticker)
147147

148148
def find_token
149149
# First, deal with claim_token
150-
if params[:t].present? && secure_compare?(params[:t], @bike.current_ownership.token)
150+
if params[:t].present? && Binxtils::Secure.compare?(params[:t], @bike.current_ownership.token)
151151
@claim_message = @bike.current_ownership&.claim_message
152152
session[:claim_token_email] = @bike.current_ownership.owner_email
153153
end

app/controllers/concerns/controller_helpers.rb

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,6 @@ def forwarded_ip_address
4242
@forwarded_ip_address ||= IpAddressParser.forwarded_address(request)
4343
end
4444

45-
# Constant-time token check. Blank expected returns false, so an unset ENV
46-
# token can't match a missing param
47-
def secure_compare?(value, expected)
48-
expected.present? && ActiveSupport::SecurityUtils.secure_compare(value.to_s, expected)
49-
end
50-
5145
def request_location_hash
5246
@request_location_hash ||= IpAddressParser.location_hash(request)
5347
end

app/controllers/organized/bulk_imports_controller.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,11 @@ def ensure_can_create_import!
8888

8989
if params[:organization_id] == "ascend"
9090
@ascend_import = true
91-
return true if secure_compare?(request.headers["Authorization"], BulkImport.ascend_api_token)
91+
return true if Binxtils::Secure.compare?(request.headers["Authorization"], BulkImport.ascend_api_token)
9292
else
9393
ensure_current_organization!
9494
@current_user = current_organization.auto_user # Crazy override to make current user work
95-
return true if secure_compare?(request.headers["Authorization"], current_organization.access_token)
95+
return true if Binxtils::Secure.compare?(request.headers["Authorization"], current_organization.access_token)
9696
end
9797
render(json: {error: "Not permitted"}, status: 401) && return
9898
end

app/controllers/register_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ def confirm_email
115115
return redirect_to_current_step if @b_param.email_confirmed?
116116

117117
if @b_param.email_confirmation_token_expired? ||
118-
!secure_compare?(params[:confirmation_token], @b_param.email_confirmation_token)
118+
!Binxtils::Secure.compare?(params[:confirmation_token], @b_param.email_confirmation_token)
119119
BikeServices::Register.send_confirmation_email(@b_param)
120120
flash[:error] = translation(:confirmation_link_expired)
121121
return redirect_to_current_step

app/controllers/strava_integrations_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ def callback
2828
return
2929
end
3030

31-
unless secure_compare?(params[:state], session.delete(:strava_oauth_state))
31+
unless Binxtils::Secure.compare?(params[:state], session.delete(:strava_oauth_state))
3232
flash[:error] = "Invalid OAuth state. Please try again."
3333
redirect_to return_to
3434
return

app/controllers/webhooks_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ def stripe
4646
private
4747

4848
def strava_verify_subscription
49-
if secure_compare?(params["hub.verify_token"], STRAVA_WEBHOOK_VERIFY_TOKEN)
49+
if Binxtils::Secure.compare?(params["hub.verify_token"], STRAVA_WEBHOOK_VERIFY_TOKEN)
5050
render json: {"hub.challenge" => params["hub.challenge"]}, status: :ok
5151
else
5252
head :forbidden

0 commit comments

Comments
 (0)