CVE-2023-0299: Improper Input Validation (CWE-20)
value: Improper Input Validation in GitHub repository publify/publify prior to 9.2.10.
CWE description: The product receives input or data, but it does
        not validate or incorrectly validates that the input has the
        properties that are required to process the data safely and
        correctly.
CVSS v3 base score: 9.8
Published: 2023-01-14T15:15Z
Repository: https://github.com/publify/publify
Language: Ruby
Files changed: publify_core/Manifest.txt, publify_core/app/models/blog.rb, publify_core/app/models/concerns/string_length_limit.rb, publify_core/app/models/content.rb, publify_core/app/models/feedback.rb, publify_core/app/models/ping.rb, publify_core/app/models/post_type.rb, publify_core/app/models/redirect.rb, publify_core/app/models/resource.rb, publify_core/app/models/tag.rb, publify_core/app/models/user.rb, publify_core/spec/models/blog_spec.rb, publify_core/spec/models/content_spec.rb, publify_core/spec/models/feedback_spec.rb, publify_core/spec/models/ping_spec.rb, publify_core/spec/models/post_type_spec.rb, publify_core/spec/models/redirect_spec.rb, publify_core/spec/models/resource_spec.rb, publify_core/spec/models/tag_spec.rb, publify_core/spec/models/user_spec.rb
Commit date: 2022-08-14 14:02:12 +0200
Commit message: Validate lengths of string attributes

When using the MySQL database, Rails automatically sets a limit of 255
characters for string columns. This changes ensures this limit is also
enforced in the validations. For several columns with a different limit,
that different limit is enforced instead.

For the PostgreSQL database, there is no limit in the database, but it
is useful to set a limit anyway to prevent absurdly large values from
being submitted.

Diff: diff --git a/publify_core/Manifest.txt b/publify_core/Manifest.txt
index 4c64d51ddf..835f7832b9 100644
--- a/publify_core/Manifest.txt
+++ b/publify_core/Manifest.txt
@@ -158,10 +158,11 @@ app/models/ability.rb
 app/models/archives_sidebar.rb
 app/models/article.rb
 app/models/article/factory.rb
 app/models/blog.rb
 app/models/comment.rb
+app/models/concerns/string_length_limit.rb
 app/models/config_manager.rb
 app/models/content.rb
 app/models/content_base.rb
 app/models/feedback.rb
 app/models/meta_sidebar.rb
diff --git a/publify_core/app/models/blog.rb b/publify_core/app/models/blog.rb
index 87cbb19805..ab5aad10b5 100644
--- a/publify_core/app/models/blog.rb
+++ b/publify_core/app/models/blog.rb
@@ -7,10 +7,12 @@
 # Once upon a time, there were plans to make publify handle multiple blogs,
 # but it never happened and publify is now firmly single-blog.
 #
 class Blog < ApplicationRecord
   include ConfigManager
+  include StringLengthLimit
+
   include Rails.application.routes.url_helpers
 
   has_many :contents
   has_many :articles
   has_many :feedback, through: :articles
@@ -137,10 +139,11 @@ class Blog < ApplicationRecord
   setting :custom_url_shortener, :string, ""
   setting :statuses_in_timeline, :boolean, true
 
   validate :permalink_has_identifier
   # validates :base_url, presence: true
+  validates_default_string_length :base_url
 
   # Find the Blog that matches a specific base URL. If no Blog object is found
   # that matches, then grab the first blog. If *that* fails, then create a new
   # Blog. The last case should only be used when Publify is first installed.
   def self.find_blog(base_url)
diff --git a/publify_core/app/models/concerns/string_length_limit.rb b/publify_core/app/models/concerns/string_length_limit.rb
new file mode 100644
index 0000000000..f005630953
--- /dev/null
+++ b/publify_core/app/models/concerns/string_length_limit.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+module StringLengthLimit
+  # Default string length limit for model attributes. When running on MySQL,
+  # this is equal to the default string length in the database as set by Rails.
+  STRING_LIMIT = 255
+
+  extend ActiveSupport::Concern
+
+  class_methods do
+    def validates_default_string_length(*names)
+      names.each do |name|
+        validates name, length: { maximum: STRING_LIMIT }
+      end
+    end
+  end
+end
diff --git a/publify_core/app/models/content.rb b/publify_core/app/models/content.rb
index f80a6dfc99..d02131f04b 100644
--- a/publify_core/app/models/content.rb
+++ b/publify_core/app/models/content.rb
@@ -3,10 +3,11 @@
 require "set"
 require "uri"
 
 class Content < ApplicationRecord
   include ContentBase
+  include StringLengthLimit
 
   belongs_to :user, optional: true, touch: true
   belongs_to :blog
 
   validates :blog, presence: true
@@ -36,10 +37,13 @@ class Content < ApplicationRecord
                               where(published_at: PublifyTime.delta_like(date_at))
                             }
 
   serialize :whiteboard
 
+  validates_default_string_length :title, :author, :permalink, :name,
+                                  :post_type, :text_filter_name
+
   def author=(user)
     if user.respond_to?(:login)
       self[:author] = user.login
       self.user = user
     elsif user.is_a?(String)
diff --git a/publify_core/app/models/feedback.rb b/publify_core/app/models/feedback.rb
index e3602e167b..2698b44c54 100644
--- a/publify_core/app/models/feedback.rb
+++ b/publify_core/app/models/feedback.rb
@@ -8,14 +8,20 @@ class Feedback < ApplicationRecord
 
   belongs_to :article, touch: true
 
   include PublifyGuid
   include ContentBase
+  include StringLengthLimit
 
   validate :feedback_allowed, on: :create
   validates :article, presence: true
 
+  validates_default_string_length :title, :author, :email, :url, :blog_name,
+                                  :user_agent, :text_filter_name
+
+  validates :ip, length: { maximum: 40 }
+
   before_save :correct_url, :classify_content
   before_create :create_guid
 
   # TODO: Rename so it doesn't sound like only approved ham
   scope :ham, -> { where(state: %w(presumed_ham ham)) }
diff --git a/publify_core/app/models/ping.rb b/publify_core/app/models/ping.rb
index bb0f89d236..a600eff7ad 100644
--- a/publify_core/app/models/ping.rb
+++ b/publify_core/app/models/ping.rb
@@ -1,5 +1,8 @@
 # frozen_string_literal: true
 
 class Ping < ApplicationRecord
+  include StringLengthLimit
+
   belongs_to :article
+  validates_default_string_length :url
 end
diff --git a/publify_core/app/models/post_type.rb b/publify_core/app/models/post_type.rb
index aed69d7463..dce6dcb769 100644
--- a/publify_core/app/models/post_type.rb
+++ b/publify_core/app/models/post_type.rb
@@ -1,11 +1,15 @@
 # frozen_string_literal: true
 
 class PostType < ApplicationRecord
+  include StringLengthLimit
+
   validates :name, uniqueness: true
   validates :name, presence: true
   validate :name_is_not_read
+  validates_default_string_length :name, :permalink, :description
+
   before_save :sanitize_title
 
   def name_is_not_read
     errors.add(:name, I18n.t("errors.article_type_already_exist")) if name == "read"
   end
diff --git a/publify_core/app/models/redirect.rb b/publify_core/app/models/redirect.rb
index 4b5ca85d70..5095ed46a2 100644
--- a/publify_core/app/models/redirect.rb
+++ b/publify_core/app/models/redirect.rb
@@ -1,15 +1,19 @@
 # frozen_string_literal: true
 
 class Redirect < ApplicationRecord
+  include StringLengthLimit
+
   belongs_to :content, optional: true, touch: true
   belongs_to :blog
 
   validates :from_path, uniqueness: true
   validates :to_path, presence: true
   validates :blog, presence: true
 
+  validates_default_string_length :from_path, :to_path
+
   def full_to_path
     path = to_path
     # FIXME: Unify HTTP URI matchers
     return path if %r{^(https?)://([^/]*)(.*)}.match?(path)
 
diff --git a/publify_core/app/models/resource.rb b/publify_core/app/models/resource.rb
index 209568405b..3d5402fde1 100644
--- a/publify_core/app/models/resource.rb
+++ b/publify_core/app/models/resource.rb
@@ -2,11 +2,14 @@
 
 require "carrierwave"
 require "carrierwave/orm/activerecord"
 
 class Resource < ApplicationRecord
+  include StringLengthLimit
   belongs_to :blog
   belongs_to :content, optional: true
 
   mount_uploader :upload, ResourceUploader
   validates :upload, presence: true
+
+  validates_default_string_length :mime
 end
diff --git a/publify_core/app/models/tag.rb b/publify_core/app/models/tag.rb
index 62a7b7e459..548b245060 100644
--- a/publify_core/app/models/tag.rb
+++ b/publify_core/app/models/tag.rb
@@ -1,14 +1,17 @@
 # frozen_string_literal: true
 
 class Tag < ApplicationRecord
+  include StringLengthLimit
+
   belongs_to :blog
   has_and_belongs_to_many :contents, order: "created_at DESC"
 
   validates :name, uniqueness: { scope: :blog_id }
   validates :blog, presence: true
   validates :name, presence: true
+  validates_default_string_length :display_name
 
   before_validation :ensure_naming_conventions
 
   attr_accessor :description, :keywords
 
diff --git a/publify_core/app/models/user.rb b/publify_core/app/models/user.rb
index ed3f366201..c9555abc18 100644
--- a/publify_core/app/models/user.rb
+++ b/publify_core/app/models/user.rb
@@ -12,16 +12,18 @@ class User < ApplicationRecord
   # Include default devise modules. Others available are:
   # :confirmable, :lockable, :timeoutable and :omniauthable
   devise :database_authenticatable, :registerable,
          :recoverable, :rememberable, :trackable, :validatable
   include ConfigManager
+  include StringLengthLimit
 
   before_validation :set_default_profile
 
   validates :login, uniqueness: true
   validates :email, :login, presence: true
   validates :login, length: { in: 3..40 }
+  validates_default_string_length :email, :text_filter_name
 
   belongs_to :resource, optional: true
   has_many :notifications, foreign_key: "notify_user_id"
   has_many :notify_contents, -> { uniq }, through: :notifications,
                                           source: "notify_content"
diff --git a/publify_core/spec/models/blog_spec.rb b/publify_core/spec/models/blog_spec.rb
index 67b53df029..c770114515 100644
--- a/publify_core/spec/models/blog_spec.rb
+++ b/publify_core/spec/models/blog_spec.rb
@@ -111,10 +111,14 @@ describe Blog, type: :model do
   end
 
   describe "validations" do
     let(:blog) { described_class.new }
 
+    it "requires base url to not be too long" do
+      expect(blog).to validate_length_of(:base_url).is_at_most(255)
+    end
+
     it "requires blog name to not be too long" do
       expect(blog).to validate_length_of(:blog_name).is_at_most(256)
     end
 
     it "allows up to 2048 characters for the rss_description_text setting" do
diff --git a/publify_core/spec/models/content_spec.rb b/publify_core/spec/models/content_spec.rb
index 35865c0c48..655160a402 100644
--- a/publify_core/spec/models/content_spec.rb
+++ b/publify_core/spec/models/content_spec.rb
@@ -142,6 +142,34 @@ describe Content, type: :model do
       let(:author) { build(:user, name: "") }
 
       it { expect(content.author_name).to eq(author.login) }
     end
   end
+
+  describe "validations" do
+    let(:content) { described_class.new }
+
+    it "requires title to not be too long" do
+      expect(content).to validate_length_of(:title).is_at_most(255)
+    end
+
+    it "requires author to not be too long" do
+      expect(content).to validate_length_of(:author).is_at_most(255)
+    end
+
+    it "requires permalink to not be too long" do
+      expect(content).to validate_length_of(:permalink).is_at_most(255)
+    end
+
+    it "requires name to not be too long" do
+      expect(content).to validate_length_of(:name).is_at_most(255)
+    end
+
+    it "requires post_type to not be too long" do
+      expect(content).to validate_length_of(:post_type).is_at_most(255)
+    end
+
+    it "requires text_filter_name to not be too long" do
+      expect(content).to validate_length_of(:text_filter_name).is_at_most(255)
+    end
+  end
 end
diff --git a/publify_core/spec/models/feedback_spec.rb b/publify_core/spec/models/feedback_spec.rb
index 62145e6091..109fa6a6ca 100644
--- a/publify_core/spec/models/feedback_spec.rb
+++ b/publify_core/spec/models/feedback_spec.rb
@@ -176,6 +176,42 @@ describe Feedback, type: :model do
       @comment[:state] = "presumed_ham"
       @comment.mark_as_spam
       assert !@comment.published?
     end
   end
+
+  describe "validations" do
+    let(:feedback) { described_class.new }
+
+    it "requires title to not be too long" do
+      expect(feedback).to validate_length_of(:title).is_at_most(255)
+    end
+
+    it "requires author to not be too long" do
+      expect(feedback).to validate_length_of(:author).is_at_most(255)
+    end
+
+    it "requires email to not be too long" do
+      expect(feedback).to validate_length_of(:email).is_at_most(255)
+    end
+
+    it "requires url to not be too long" do
+      expect(feedback).to validate_length_of(:url).is_at_most(255)
+    end
+
+    it "requires ip to not be too long" do
+      expect(feedback).to validate_length_of(:ip).is_at_most(40)
+    end
+
+    it "requires blog_name to not be too long" do
+      expect(feedback).to validate_length_of(:blog_name).is_at_most(255)
+    end
+
+    it "requires user_agent to not be too long" do
+      expect(feedback).to validate_length_of(:user_agent).is_at_most(255)
+    end
+
+    it "requires text_filter_name to not be too long" do
+      expect(feedback).to validate_length_of(:text_filter_name).is_at_most(255)
+    end
+  end
 end
diff --git a/publify_core/spec/models/ping_spec.rb b/publify_core/spec/models/ping_spec.rb
index 7b4b0dc56e..03f48f5c4d 100644
--- a/publify_core/spec/models/ping_spec.rb
+++ b/publify_core/spec/models/ping_spec.rb
@@ -1,6 +1,13 @@
 # frozen_string_literal: true
 
 require "rails_helper"
 
 describe Ping, type: :model do
+  describe "validations" do
+    let(:ping) { described_class.new }
+
+    it "requires url to not be too long" do
+      expect(ping).to validate_length_of(:url).is_at_most(255)
+    end
+  end
 end
diff --git a/publify_core/spec/models/post_type_spec.rb b/publify_core/spec/models/post_type_spec.rb
index 4efe28a310..fa423182e5 100644
--- a/publify_core/spec/models/post_type_spec.rb
+++ b/publify_core/spec/models/post_type_spec.rb
@@ -27,6 +27,22 @@ describe PostType, type: :model do
     expect { described_class.create!(name: "test") }.not_to raise_error
     test_type = described_class.new(name: "test")
     expect(test_type).not_to be_valid
     expect(test_type.errors[:name]).to eq(["has already been taken"])
   end
+
+  describe "validations" do
+    let(:post_type) { described_class.new }
+
+    it "requires name to not be too long" do
+      expect(post_type).to validate_length_of(:name).is_at_most(255)
+    end
+
+    it "requires permalink to not be too long" do
+      expect(post_type).to validate_length_of(:permalink).is_at_most(255)
+    end
+
+    it "requires description to not be too long" do
+      expect(post_type).to validate_length_of(:description).is_at_most(255)
+    end
+  end
 end
diff --git a/publify_core/spec/models/redirect_spec.rb b/publify_core/spec/models/redirect_spec.rb
index 0c5f303e43..8e093e74ec 100644
--- a/publify_core/spec/models/redirect_spec.rb
+++ b/publify_core/spec/models/redirect_spec.rb
@@ -18,6 +18,18 @@ describe Redirect, type: :model do
     it "is based on the blog's base_url" do
       redirect = blog.redirects.build(from_path: "right/here", to_path: "over_there")
       expect(redirect.from_url).to eq "#{blog.base_url}/right/here"
     end
   end
+
+  describe "validations" do
+    let(:redirect) { described_class.new }
+
+    it "requires from_path to not be too long" do
+      expect(redirect).to validate_length_of(:from_path).is_at_most(255)
+    end
+
+    it "requires to_path to not be too long" do
+      expect(redirect).to validate_length_of(:to_path).is_at_most(255)
+    end
+  end
 end
diff --git a/publify_core/spec/models/resource_spec.rb b/publify_core/spec/models/resource_spec.rb
index 692d3924d1..44a60875e2 100644
--- a/publify_core/spec/models/resource_spec.rb
+++ b/publify_core/spec/models/resource_spec.rb
@@ -31,6 +31,14 @@ describe Resource, type: :model do
 
     it "gives the correct url for the image versions" do
       expect(img_resource.upload_url(:thumb)).to eq "/files/resource/1/thumb_testfile.png"
     end
   end
+
+  describe "validations" do
+    let(:resource) { described_class.new }
+
+    it "requires mime to not be too long" do
+      expect(resource).to validate_length_of(:mime).is_at_most(255)
+    end
+  end
 end
diff --git a/publify_core/spec/models/tag_spec.rb b/publify_core/spec/models/tag_spec.rb
index 266c8ae339..5c821e6fd2 100644
--- a/publify_core/spec/models/tag_spec.rb
+++ b/publify_core/spec/models/tag_spec.rb
@@ -10,13 +10,19 @@ describe Tag, type: :model do
     test_tag = blog.tags.new(name: "test")
     expect(test_tag).not_to be_valid
     expect(test_tag.errors[:name]).to eq(["has already been taken"])
   end
 
-  describe "validation" do
+  describe "validations" do
+    let(:tag) { described_class.new }
+
     it "requires name to be present" do
-      expect(blog.tags.build(name: "")).not_to be_valid
+      expect(tag).to validate_presence_of(:name)
+    end
+
+    it "requires display_name to not be too long" do
+      expect(tag).to validate_length_of(:display_name).is_at_most(255)
     end
   end
 
   it "display names with spaces can be found by dash joined name" do
     tag = blog.tags.create(name: "Monty Python")
diff --git a/publify_core/spec/models/user_spec.rb b/publify_core/spec/models/user_spec.rb
index fe3c0e44bf..a8457ef283 100644
--- a/publify_core/spec/models/user_spec.rb
+++ b/publify_core/spec/models/user_spec.rb
@@ -43,10 +43,14 @@ describe User, type: :model do
   end
 
   describe "validations" do
     let(:user) { described_class.new }
 
+    it "requires email to not be too long" do
+      expect(user).to validate_length_of(:email).is_at_most(255)
+    end
+
     it "requires first name to not be too long" do
       expect(user).to validate_length_of(:firstname).is_at_most(256)
     end
 
     it "requires last name to not be too long" do
@@ -71,10 +75,14 @@ describe User, type: :model do
 
     it "requires the login field to be present" do
       expect(user).to validate_presence_of(:login)
     end
 
+    it "requires text_filter_name to not be too long" do
+      expect(user).to validate_length_of(:text_filter_name).is_at_most(255)
+    end
+
     it "does not allow duplicate logins when updating a user" do
       create :user, login: "foo"
       bar = create :user, login: "bar"
 
       expect(bar).not_to allow_value("foo").for(:login)

Vulnerable code:   describe "validation" do
      expect(blog.tags.build(name: "")).not_to be_valid
Fixed code: app/models/concerns/string_length_limit.rb
  include StringLengthLimit

  validates_default_string_length :base_url
# frozen_string_literal: true

module StringLengthLimit
  # Default string length limit for model attributes. When running on MySQL,
  # this is equal to the default string length in the database as set by Rails.
  STRING_LIMIT = 255

  extend ActiveSupport::Concern

  class_methods do
    def validates_default_string_length(*names)
      names.each do |name|
        validates name, length: { maximum: STRING_LIMIT }
      end
    end
  end
end
  include StringLengthLimit
  validates_default_string_length :title, :author, :permalink, :name,
                                  :post_type, :text_filter_name

  include StringLengthLimit
  validates_default_string_length :title, :author, :email, :url, :blog_name,
                                  :user_agent, :text_filter_name

  validates :ip, length: { maximum: 40 }

  include StringLengthLimit

  validates_default_string_length :url
  include StringLengthLimit

  validates_default_string_length :name, :permalink, :description

  include StringLengthLimit

  validates_default_string_length :from_path, :to_path

  include StringLengthLimit

  validates_default_string_length :mime
  include StringLengthLimit

  validates_default_string_length :display_name
  include StringLengthLimit
  validates_default_string_length :email, :text_filter_name
    it "requires base url to not be too long" do
      expect(blog).to validate_length_of(:base_url).is_at_most(255)
    end


  describe "validations" do
    let(:content) { described_class.new }

    it "requires title to not be too long" do
      expect(content).to validate_length_of(:title).is_at_most(255)
    end

    it "requires author to not be too long" do
      expect(content).to validate_length_of(:author).is_at_most(255)
    end

    it "requires permalink to not be too long" do
      expect(content).to validate_length_of(:permalink).is_at_most(255)
    end

    it "requires name to not be too long" do
      expect(content).to validate_length_of(:name).is_at_most(255)
    end

    it "requires post_type to not be too long" do
      expect(content).to validate_length_of(:post_type).is_at_most(255)
    end

    it "requires text_filter_name to not be too long" do
      expect(content).to validate_length_of(:text_filter_name).is_at_most(255)
    end
  end

  describe "validations" do
    let(:feedback) { described_class.new }

    it "requires title to not be too long" do
      expect(feedback).to validate_length_of(:title).is_at_most(255)
    end

    it "requires author to not be too long" do
      expect(feedback).to validate_length_of(:author).is_at_most(255)
    end

    it "requires email to not be too long" do
      expect(feedback).to validate_length_of(:email).is_at_most(255)
    end

    it "requires url to not be too long" do
      expect(feedback).to validate_length_of(:url).is_at_most(255)
    end

    it "requires ip to not be too long" do
      expect(feedback).to validate_length_of(:ip).is_at_most(40)
    end

    it "requires blog_name to not be too long" do
      expect(feedback).to validate_length_of(:blog_name).is_at_most(255)
    end

    it "requires user_agent to not be too long" do
      expect(feedback).to validate_length_of(:user_agent).is_at_most(255)
    end

    it "requires text_filter_name to not be too long" do
      expect(feedback).to validate_length_of(:text_filter_name).is_at_most(255)
    end
  end
  describe "validations" do
    let(:ping) { described_class.new }

    it "requires url to not be too long" do
      expect(ping).to validate_length_of(:url).is_at_most(255)
    end
  end

  describe "validations" do
    let(:post_type) { described_class.new }

    it "requires name to not be too long" do
      expect(post_type).to validate_length_of(:name).is_at_most(255)
    end

    it "requires permalink to not be too long" do
      expect(post_type).to validate_length_of(:permalink).is_at_most(255)
    end

    it "requires description to not be too long" do
      expect(post_type).to validate_length_of(:description).is_at_most(255)
    end
  end

  describe "validations" do
    let(:redirect) { described_class.new }

    it "requires from_path to not be too long" do
      expect(redirect).to validate_length_of(:from_path).is_at_most(255)
    end

    it "requires to_path to not be too long" do
      expect(redirect).to validate_length_of(:to_path).is_at_most(255)
    end
  end

  describe "validations" do
    let(:resource) { described_class.new }

    it "requires mime to not be too long" do
      expect(resource).to validate_length_of(:mime).is_at_most(255)
    end
  end
  describe "validations" do
    let(:tag) { described_class.new }

      expect(tag).to validate_presence_of(:name)
    end

    it "requires display_name to not be too long" do
      expect(tag).to validate_length_of(:display_name).is_at_most(255)
    it "requires email to not be too long" do
      expect(user).to validate_length_of(:email).is_at_most(255)
    end

    it "requires text_filter_name to not be too long" do
      expect(user).to validate_length_of(:text_filter_name).is_at_most(255)
    end