CVE-2023-0569: Weak Password Requirements (CWE-521)
value: Weak Password Requirements in GitHub repository publify/publify prior to 9.2.10.
CWE description: The product does not require that users should have strong passwords, which makes it easier for attackers to compromise user accounts.
CVSS v3 base score: 6.5
Published: 2023-01-29T17:15Z
Repository: https://github.com/publify/publify
Language: Ruby
Files changed: publify_core/app/models/user.rb, publify_core/lib/publify_core.rb, publify_core/lib/publify_core/testing_support/factories.rb, publify_core/publify_core.gemspec, publify_core/spec/controllers/admin/users_controller_spec.rb, publify_core/spec/controllers/setup_controller_spec.rb, publify_core/spec/features/login_spec.rb, publify_core/spec/features/reset_password_spec.rb, publify_core/spec/features/setup_spec.rb, publify_core/spec/features/signup_spec.rb, publify_core/spec/models/user_spec.rb
Commit date: 2022-08-19 23:41:41 +0200
Commit message: Require user passwords to be strong

This adds validation of passwords using the zxcvbn password strength
estimation gem.

Diff: diff --git a/publify_core/app/models/user.rb b/publify_core/app/models/user.rb
index 226183e3c9..5b938466a2 100644
--- a/publify_core/app/models/user.rb
+++ b/publify_core/app/models/user.rb
@@ -10,11 +10,11 @@ class User < ApplicationRecord
   CONTRIBUTOR = "contributor"
 
   # Include default devise modules. Others available are:
   # :confirmable, :lockable, :timeoutable and :omniauthable
   devise :database_authenticatable, :registerable,
-         :recoverable, :rememberable, :trackable, :validatable
+         :recoverable, :rememberable, :trackable, :validatable, :zxcvbnable
   include ConfigManager
   include StringLengthLimit
 
   before_validation :set_default_profile
 
diff --git a/publify_core/lib/publify_core.rb b/publify_core/lib/publify_core.rb
index b1071e352e..e482c29e7e 100644
--- a/publify_core/lib/publify_core.rb
+++ b/publify_core/lib/publify_core.rb
@@ -1,9 +1,10 @@
 # frozen_string_literal: true
 
 require "devise"
 require "devise-i18n"
+require "devise_zxcvbn"
 
 require "publify_core/version"
 require "publify_core/engine"
 require "publify_core/lang"
 
diff --git a/publify_core/lib/publify_core/testing_support/factories.rb b/publify_core/lib/publify_core/testing_support/factories.rb
index a44df09815..8eacf71372 100644
--- a/publify_core/lib/publify_core/testing_support/factories.rb
+++ b/publify_core/lib/publify_core/testing_support/factories.rb
@@ -19,11 +19,11 @@ FactoryBot.define do
     name { "Bond" }
     nickname { "James Bond" }
     notify_via_email { false }
     notify_on_new_articles { false }
     notify_on_comments { false }
-    password { "top-secret" }
+    password { "top-Secret12!$#" }
     state { "active" }
     profile { User::CONTRIBUTOR }
 
     trait :without_twitter do
       twitter { nil }
diff --git a/publify_core/publify_core.gemspec b/publify_core/publify_core.gemspec
index b4c3234665..13cbb40b67 100644
--- a/publify_core/publify_core.gemspec
+++ b/publify_core/publify_core.gemspec
@@ -26,10 +26,11 @@ Gem::Specification.new do |s|
   s.add_dependency "bluecloth", "~> 2.1"
   s.add_dependency "cancancan", "~> 3.0"
   s.add_dependency "carrierwave", "~> 2.0"
   s.add_dependency "devise", "~> 4.7.1"
   s.add_dependency "devise-i18n", "~> 1.2"
+  s.add_dependency "devise_zxcvbn", "~> 6.0"
   s.add_dependency "dynamic_form", "~> 1.1.4"
   s.add_dependency "fog-aws", "~> 3.2"
   s.add_dependency "fog-core", "~> 2.2"
   s.add_dependency "jquery-rails", "~> 4.4.0"
   s.add_dependency "jquery-ui-rails", "~> 6.0.1"
diff --git a/publify_core/spec/controllers/admin/users_controller_spec.rb b/publify_core/spec/controllers/admin/users_controller_spec.rb
index 5a947850d2..b14ab92290 100644
--- a/publify_core/spec/controllers/admin/users_controller_spec.rb
+++ b/publify_core/spec/controllers/admin/users_controller_spec.rb
@@ -5,10 +5,11 @@ require "rails_helper"
 describe Admin::UsersController, type: :controller do
   let!(:blog) { create :blog }
   let(:admin) { create(:user, :as_admin) }
   let(:publisher) { create(:user, :as_publisher) }
   let(:contributor) { create(:user, :as_contributor) }
+  let(:strong_password) { "fhnehnhfiiuh" }
 
   render_views
 
   describe "#index" do
     let(:user) { admin }
@@ -46,12 +47,12 @@ describe Admin::UsersController, type: :controller do
 
   describe "#create" do
     before do
       sign_in admin
       post :create, params: { user: { login: "errand", email: "corey@test.com",
-                                      password: "testpass",
-                                      password_confirmation: "testpass",
+                                      password: strong_password,
+                                      password_confirmation: strong_password,
                                       profile: User::CONTRIBUTOR,
                                       text_filter_name: "markdown",
                                       nickname: "fooo", firstname: "bar" } }
     end
 
@@ -68,12 +69,13 @@ describe Admin::UsersController, type: :controller do
     end
 
     it "redirects to index" do
       post :update, params: { id: contributor.id,
                               user: { login: "errand",
-                                      email: "corey@test.com", password: "testpass",
-                                      password_confirmation: "testpass" } }
+                                      email: "corey@test.com",
+                                      password: strong_password,
+                                      password_confirmation: strong_password } }
       expect(response).to redirect_to(action: "index")
     end
 
     it "skips blank passwords" do
       post :update, params: { id: contributor.id,
diff --git a/publify_core/spec/controllers/setup_controller_spec.rb b/publify_core/spec/controllers/setup_controller_spec.rb
index d2bad1964a..6713707ab0 100644
--- a/publify_core/spec/controllers/setup_controller_spec.rb
+++ b/publify_core/spec/controllers/setup_controller_spec.rb
@@ -1,10 +1,12 @@
 # frozen_string_literal: true
 
 require "rails_helper"
 
 describe SetupController, type: :controller do
+  let(:strong_password) { "fhnehnhfiiuh" }
+
   describe "#index" do
     describe "when no blog is configured" do
       before do
         # Set up database similar to result of seeding
         @blog = Blog.create
@@ -33,11 +35,11 @@ describe SetupController, type: :controller do
 
       context "when passing correct parameters" do
         before do
           ActionMailer::Base.deliveries.clear
           post :create, params: { setting: { blog_name: "Foo", email: "foo@bar.net",
-                                             password: "foo123bar" } }
+                                             password: strong_password } }
         end
 
         it "correctly initializes blog and users" do
           expect(Blog.first.blog_name).to eq("Foo")
           admin = User.find_by(login: "admin")
@@ -62,25 +64,31 @@ describe SetupController, type: :controller do
       end
 
       describe "when passing incorrect parameters" do
         it "empty blog name should raise an error" do
           post :create, params: { setting: { blog_name: "", email: "foo@bar.net",
-                                             password: "foobar123" } }
+                                             password: strong_password } }
           expect(response).to redirect_to(action: "index")
         end
 
         it "empty email should raise an error" do
           post :create, params: { setting: { blog_name: "Foo", email: "",
-                                             password: "foobar123" } }
+                                             password: strong_password } }
           expect(response).to redirect_to(action: "index")
         end
 
         it "empty password should raise an error" do
           post :create, params: { setting: { blog_name: "Foo", email: "foo@bar.net",
                                              password: "" } }
           expect(response).to redirect_to(action: "index")
         end
+
+        it "weak password should raise an error" do
+          post :create, params: { setting: { blog_name: "Foo", email: "foo@bar.net",
+                                             password: "foo123bar" } }
+          expect(response).to redirect_to(action: "index")
+        end
       end
     end
 
     describe "when a blog is configured and has some users" do
       before do
diff --git a/publify_core/spec/features/login_spec.rb b/publify_core/spec/features/login_spec.rb
index e0947b34ae..238433c09f 100644
--- a/publify_core/spec/features/login_spec.rb
+++ b/publify_core/spec/features/login_spec.rb
@@ -1,23 +1,25 @@
 # frozen_string_literal: true
 
 require "rails_helper"
 
 RSpec.feature "Logging in", type: :feature do
+  let(:strong_password) { "fhnehnhfiiuh" }
+
   before do
     stub_request(:get,
                  "http://www.google.com/search?output=rss&q=link:www.example.com&tbm=blg").
       to_return(status: 200, body: "", headers: {})
     load Rails.root.join("db/seeds.rb")
     Blog.first.update blog_name: "Awesome!", base_url: "http://www.example.com/"
-    create :user, :as_admin, login: "admin", password: "a-secret"
+    create :user, :as_admin, login: "admin", password: strong_password
   end
 
   scenario "Admin logs in" do
     visit "/admin"
     fill_in :user_login, with: "admin"
-    fill_in :user_password, with: "a-secret"
+    fill_in :user_password, with: strong_password
 
     click_button I18n.t("devise.sessions.new.sign_in")
 
     expect(page).to have_text I18n.t("devise.sessions.signed_in")
   end
diff --git a/publify_core/spec/features/reset_password_spec.rb b/publify_core/spec/features/reset_password_spec.rb
index b97f746cac..23c09ceea3 100644
--- a/publify_core/spec/features/reset_password_spec.rb
+++ b/publify_core/spec/features/reset_password_spec.rb
@@ -4,11 +4,11 @@ require "rails_helper"
 
 RSpec.feature "Logging in", type: :feature do
   before do
     load Rails.root.join("db/seeds.rb")
     Blog.first.update blog_name: "Awesome!", base_url: "http://www.example.com/"
-    create :user, :as_admin, login: "admin", password: "forget-me"
+    create :user, :as_admin, login: "admin", password: "Fo_rgEt-1m5e2"
   end
 
   scenario "Admin resets password" do
     visit "/admin"
     click_link I18n.t("accounts.lost_my_password")
@@ -21,12 +21,12 @@ RSpec.feature "Logging in", type: :feature do
     link = html.at_css "a"
     url = link.attribute("href").value
 
     visit url
 
-    fill_in :user_password, with: "a-secret"
-    fill_in :user_password_confirmation, with: "a-secret"
+    fill_in :user_password, with: "a5-SeCre1T4"
+    fill_in :user_password_confirmation, with: "a5-SeCre1T4"
 
     click_button I18n.t("devise.passwords.edit.change_my_password")
     expect(page).to have_text I18n.t("devise.passwords.updated")
   end
 end
diff --git a/publify_core/spec/features/setup_spec.rb b/publify_core/spec/features/setup_spec.rb
index 1232e9ff7f..9a3c414fb7 100644
--- a/publify_core/spec/features/setup_spec.rb
+++ b/publify_core/spec/features/setup_spec.rb
@@ -1,10 +1,12 @@
 # frozen_string_literal: true
 
 require "rails_helper"
 
 RSpec.feature "Blog setup", type: :feature do
+  let(:strong_password) { "fhnehnhfiiuh" }
+
   before do
     stub_request(:get,
                  "http://www.google.com/search?output=rss&q=link:www.example.com&tbm=blg").
       to_return(status: 200, body: "", headers: {})
     load Rails.root.join("db/seeds.rb")
@@ -16,11 +18,11 @@ RSpec.feature "Blog setup", type: :feature do
     expect(page).to have_text I18n.t!("setup.index.welcome_to_your_blog_setup")
 
     # Set up the blog
     fill_in :setting_blog_name, with: "Awesome blog"
     fill_in :setting_email, with: "foo@bar.com"
-    fill_in :setting_password, with: "test1234"
+    fill_in :setting_password, with: strong_password
     click_button I18n.t!("generic.save")
 
     # Confirm set up success
     expect(page).to have_text I18n.t!("accounts.confirm.success")
     expect(page).to have_text I18n.t!("accounts.confirm.login", login: "admin")
@@ -36,11 +38,11 @@ RSpec.feature "Blog setup", type: :feature do
     visit admin_dashboard_path
     find("a[href=\"#{destroy_user_session_path}\"]").click
 
     visit admin_dashboard_path
     fill_in :user_login, with: "admin"
-    fill_in :user_password, with: "test1234"
+    fill_in :user_password, with: strong_password
     find("input[type=submit]").click
     expect(page).to have_text I18n.t!("admin.dashboard.index.welcome_back",
                                       user_name: "admin")
 
     # Confirm proper setting fo user properties
diff --git a/publify_core/spec/features/signup_spec.rb b/publify_core/spec/features/signup_spec.rb
index 5265f9873f..7b576bd0e8 100644
--- a/publify_core/spec/features/signup_spec.rb
+++ b/publify_core/spec/features/signup_spec.rb
@@ -1,27 +1,29 @@
 # frozen_string_literal: true
 
 require "rails_helper"
 
 RSpec.feature "Signing up", type: :feature do
+  let(:strong_password) { "whhefwhfwefhiu" }
+
   before do
     load Rails.root.join("db/seeds.rb")
     Blog.first.update(blog_name: "Awesome!",
                       base_url: "http://www.example.com/",
                       allow_signup: 1)
-    create :user, :as_admin, login: "admin", password: "a-secret"
+    create :user, :as_admin, login: "admin"
   end
 
   scenario "User signs up for an account" do
     visit admin_dashboard_path
     click_link I18n.t("accounts.create_account")
 
     # Create account
     fill_in :user_login, with: "hello"
     fill_in :user_email, with: "hello@hello.com"
-    fill_in :user_password, with: "hush-hush"
-    fill_in :user_password_confirmation, with: "hush-hush"
+    fill_in :user_password, with: strong_password
+    fill_in :user_password_confirmation, with: strong_password
     click_button I18n.t!("devise.registrations.new.sign_up")
 
     # Confirm creation success
     expect(page).to have_text I18n.t!("devise.registrations.signed_up")
 
@@ -30,11 +32,11 @@ RSpec.feature "Signing up", type: :feature do
     find("a[href=\"#{destroy_user_session_path}\"]").click
 
     # Confirm ability to sign in
     visit admin_dashboard_path
     fill_in :user_login, with: "hello"
-    fill_in :user_password, with: "hush-hush"
+    fill_in :user_password, with: strong_password
     find("input[type=submit]").click
     expect(page).to have_text I18n.t!("devise.sessions.signed_in")
 
     # Confirm proper setting fo user properties
     u = User.last
diff --git a/publify_core/spec/models/user_spec.rb b/publify_core/spec/models/user_spec.rb
index 54ea166556..8ff5c217b0 100644
--- a/publify_core/spec/models/user_spec.rb
+++ b/publify_core/spec/models/user_spec.rb
@@ -96,10 +96,15 @@ describe User, type: :model do
       create :user, email: "foo@foo.com"
       bar = create :user, email: "bar@bar.com"
 
       expect(bar).not_to allow_value("foo@foo.com").for(:email)
     end
+
+    it "requires a strong password" do
+      nearly_valid_user = build(:user)
+      expect(nearly_valid_user).not_to allow_value("password01").for(:password)
+    end
   end
 
   describe "#initialize" do
     it "accepts a settings field in its parameter hash" do
       described_class.new("firstname" => "foo")

Vulnerable code:          :recoverable, :rememberable, :trackable, :validatable
    password { "top-secret" }
                                      password: "testpass",
                                      password_confirmation: "testpass",
                                      email: "corey@test.com", password: "testpass",
                                      password_confirmation: "testpass" } }
                                             password: "foo123bar" } }
                                             password: "foobar123" } }
                                             password: "foobar123" } }
    create :user, :as_admin, login: "admin", password: "a-secret"
    fill_in :user_password, with: "a-secret"
    create :user, :as_admin, login: "admin", password: "forget-me"
    fill_in :user_password, with: "a-secret"
    fill_in :user_password_confirmation, with: "a-secret"
    fill_in :setting_password, with: "test1234"
    fill_in :user_password, with: "test1234"
    create :user, :as_admin, login: "admin", password: "a-secret"
    fill_in :user_password, with: "hush-hush"
    fill_in :user_password_confirmation, with: "hush-hush"
    fill_in :user_password, with: "hush-hush"
Fixed code:          :recoverable, :rememberable, :trackable, :validatable, :zxcvbnable
require "devise_zxcvbn"
    password { "top-Secret12!$#" }
  s.add_dependency "devise_zxcvbn", "~> 6.0"
  let(:strong_password) { "fhnehnhfiiuh" }
                                      password: strong_password,
                                      password_confirmation: strong_password,
                                      email: "corey@test.com",
                                      password: strong_password,
                                      password_confirmation: strong_password } }
  let(:strong_password) { "fhnehnhfiiuh" }

                                             password: strong_password } }
                                             password: strong_password } }
                                             password: strong_password } }

        it "weak password should raise an error" do
          post :create, params: { setting: { blog_name: "Foo", email: "foo@bar.net",
                                             password: "foo123bar" } }
          expect(response).to redirect_to(action: "index")
        end
  let(:strong_password) { "fhnehnhfiiuh" }

    create :user, :as_admin, login: "admin", password: strong_password
    fill_in :user_password, with: strong_password
    create :user, :as_admin, login: "admin", password: "Fo_rgEt-1m5e2"
    fill_in :user_password, with: "a5-SeCre1T4"
    fill_in :user_password_confirmation, with: "a5-SeCre1T4"
  let(:strong_password) { "fhnehnhfiiuh" }

    fill_in :setting_password, with: strong_password
    fill_in :user_password, with: strong_password
  let(:strong_password) { "whhefwhfwefhiu" }

    create :user, :as_admin, login: "admin"
    fill_in :user_password, with: strong_password
    fill_in :user_password_confirmation, with: strong_password
    fill_in :user_password, with: strong_password

    it "requires a strong password" do
      nearly_valid_user = build(:user)
      expect(nearly_valid_user).not_to allow_value("password01").for(:password)
    end