Skip to content

Conversation

@pboling
Copy link
Member

@pboling pboling commented Mar 25, 2025

No description provided.

@pboling pboling self-assigned this Mar 25, 2025
def update
respond_to do |format|
if site.update_attributes(site_params)
if site.update(site_params)

Check failure

Code scanning / CodeQL

Insecure Mass Assignment Critical

This mass assignment operation can assign user-controlled attributes from
this remote flow source
.

Copilot Autofix

AI 8 months ago

To fix the problem, we need to explicitly specify which keys are permitted within the properties attribute. This can be done by listing the allowed keys in the permit method. This change ensures that only the specified keys can be assigned, preventing arbitrary parameters from being set by the user.

The best way to fix the problem without changing existing functionality is to update the site_params method to include a list of permitted keys for the properties attribute. This change should be made in the app/controllers/masq/sites_controller.rb file.

Suggested changeset 1
app/controllers/masq/sites_controller.rb

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/app/controllers/masq/sites_controller.rb b/app/controllers/masq/sites_controller.rb
--- a/app/controllers/masq/sites_controller.rb
+++ b/app/controllers/masq/sites_controller.rb
@@ -56,3 +56,3 @@
     def site_params
-      params.require(:site).permit(:persona_id, :url, properties: {})
+      params.require(:site).permit(:persona_id, :url, properties: [:key1, :key2, :key3])
     end
EOF
@@ -56,3 +56,3 @@
def site_params
params.require(:site).permit(:persona_id, :url, properties: {})
params.require(:site).permit(:persona_id, :url, properties: [:key1, :key2, :key3])
end
Copilot is powered by AI and may make mistakes. Always verify output.
@codecov
Copy link

codecov bot commented Mar 25, 2025

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️


# Encrypts some data with the salt.
def encrypt(password, salt)
Digest::SHA1.hexdigest("--#{salt}--#{password}--")

Check failure

Code scanning / CodeQL

Use of a broken or weak cryptographic hashing algorithm on sensitive data High

Sensitive data (password)
is used in a hashing algorithm (SHA1) that is insecure for password hashing, since it is not a computationally expensive hash function.

Copilot Autofix

AI 8 months ago

To fix the problem, we need to replace the use of SHA-1 with a more secure and computationally expensive hashing algorithm. Argon2 is a good choice for password hashing as it is designed to be secure and resistant to brute-force attacks. We will use the argon2 gem to implement this.

Steps to fix:

  1. Install the argon2 gem if it is not already installed.
  2. Update the encrypt method to use Argon2 for hashing passwords.
  3. Ensure that the authenticate method verifies passwords using Argon2.
Suggested changeset 2
app/models/masq/account.rb

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/app/models/masq/account.rb b/app/models/masq/account.rb
--- a/app/models/masq/account.rb
+++ b/app/models/masq/account.rb
@@ -1,2 +1,2 @@
-require "digest/sha1"
+require "argon2"
 
@@ -73,3 +73,3 @@
         if !a.nil? && a.active? && a.enabled
-          if a.authenticated?(password) || (Masq::Engine.config.masq["trust_basic_auth"] && basic_auth_used)
+          if Argon2::Password.verify_password("--#{a.salt}--#{password}--", a.encrypted_password) || (Masq::Engine.config.masq["trust_basic_auth"] && basic_auth_used)
             a.last_authenticated_at = Time.now.utc
@@ -84,3 +84,3 @@
       def encrypt(password, salt)
-        Digest::SHA1.hexdigest("--#{salt}--#{password}--")
+        Argon2::Password.create("--#{salt}--#{password}--")
       end
EOF
@@ -1,2 +1,2 @@
require "digest/sha1"
require "argon2"

@@ -73,3 +73,3 @@
if !a.nil? && a.active? && a.enabled
if a.authenticated?(password) || (Masq::Engine.config.masq["trust_basic_auth"] && basic_auth_used)
if Argon2::Password.verify_password("--#{a.salt}--#{password}--", a.encrypted_password) || (Masq::Engine.config.masq["trust_basic_auth"] && basic_auth_used)
a.last_authenticated_at = Time.now.utc
@@ -84,3 +84,3 @@
def encrypt(password, salt)
Digest::SHA1.hexdigest("--#{salt}--#{password}--")
Argon2::Password.create("--#{salt}--#{password}--")
end
Gemfile
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/Gemfile b/Gemfile
--- a/Gemfile
+++ b/Gemfile
@@ -50 +50,3 @@
 gem "rails", "~> 8.0", ">= 8.0.2"
+
+gem "argon2", "2.3.2"
EOF
@@ -50 +50,3 @@
gem "rails", "~> 8.0", ">= 8.0.2"

gem "argon2", "2.3.2"
This fix introduces these dependencies
Package Version Security advisories
argon2 (rubygems) 2.3.2 None
Copilot is powered by AI and may make mistakes. Always verify output.
@pboling
Copy link
Member Author

pboling commented Mar 26, 2025

Closing in favor of #5

@pboling pboling closed this Mar 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants