From 1ca02121976b9d06db02c17fcb51f5d31d185b0d Mon Sep 17 00:00:00 2001 From: Petr Date: Thu, 25 Jun 2026 21:45:47 +0200 Subject: [PATCH] security: add slug validation, username validation, and template escaping - Enforce SLUG_REGEX (alphanumeric + dot/dash/underscore only) in creation_error to prevent path traversal (C-1) and stored XSS (C-3). - Add File.expand_path containment check in default_path_for as defense-in-depth. - Escape slug in form action attributes (list, form views). - Validate username in change_username using the same regex as create_user (C-4). - Escape username in settings form action attributes. - Treat submitting the current username as a no-op success instead of 'taken'. --- lib/volumen/server.rb | 7 ++++++ lib/volumen/store.rb | 7 +++++- lib/volumen/web/views/form.erb | 2 +- lib/volumen/web/views/list.erb | 4 ++-- lib/volumen/web/views/settings.erb | 4 ++-- test/admin_test.rb | 34 ++++++++++++++++++++++++++++++ test/store_test.rb | 6 ++++++ 7 files changed, 58 insertions(+), 6 deletions(-) diff --git a/lib/volumen/server.rb b/lib/volumen/server.rb index 2f0b91a..7aaf77e 100644 --- a/lib/volumen/server.rb +++ b/lib/volumen/server.rb @@ -17,6 +17,7 @@ module Volumen class Server < Sinatra::Base MAX_LOGIN_ATTEMPTS = 10 LOGIN_WINDOW = 60 # seconds + SLUG_REGEX = /\A[a-z0-9](?:[a-z0-9._-]*[a-z0-9])?\z/ register ApiRoutes register AdminRoutes @@ -260,6 +261,11 @@ module Volumen def change_username new_name = presence(params["username"]) return render_settings(error: "Username cannot be empty.") if new_name.nil? + unless new_name.match?(/\A[a-zA-Z0-9._-]+\z/) + return render_settings(error: "Username may use letters, numbers, dot, dash, underscore.") + end + + return render_settings(notice: "Username unchanged.") if new_name == current_user if users.rename(current_user, new_name) session[:user] = new_name @@ -335,6 +341,7 @@ module Volumen def creation_error(post) return "Slug is required." if presence(post.slug).nil? + return "Invalid slug." unless post.slug.match?(SLUG_REGEX) return "A post with that slug already exists." if store.find(post.slug) nil diff --git a/lib/volumen/store.rb b/lib/volumen/store.rb index c729197..ef74b27 100644 --- a/lib/volumen/store.rb +++ b/lib/volumen/store.rb @@ -85,7 +85,12 @@ module Volumen end def default_path_for(post) - File.join(@content_dir, "#{post.slug}.md") + target = File.join(@content_dir, "#{post.slug}.md") + unless File.expand_path(target).start_with?(File.expand_path(@content_dir) + File::SEPARATOR) + raise ArgumentError, "slug escapes content directory" + end + + target end def safe_media_name(original) diff --git a/lib/volumen/web/views/form.erb b/lib/volumen/web/views/form.erb index 2848300..2788bdf 100644 --- a/lib/volumen/web/views/form.erb +++ b/lib/volumen/web/views/form.erb @@ -5,7 +5,7 @@ <% end %>
"> + action="<%= @mode == :new ? to("/admin/posts") : h(to("/admin/posts/#{@post.slug}")) %>">
diff --git a/lib/volumen/web/views/list.erb b/lib/volumen/web/views/list.erb index 8b580e9..a6f363a 100644 --- a/lib/volumen/web/views/list.erb +++ b/lib/volumen/web/views/list.erb @@ -19,9 +19,9 @@ <%= h(post.date_string) %> <%= post.draft? ? "yes" : "" %> - ">Edit + ">Edit " + action="<%= h(to("/admin/posts/#{post.slug}/delete")) %>" onsubmit="return confirm('Delete this post?');"> diff --git a/lib/volumen/web/views/settings.erb b/lib/volumen/web/views/settings.erb index b0bb5bb..72b0d94 100644 --- a/lib/volumen/web/views/settings.erb +++ b/lib/volumen/web/views/settings.erb @@ -41,7 +41,7 @@ <%= h(user.role) %> <% else %> "> + action="<%= h(to("/admin/settings/users/#{user.username}/role")) %>"> diff --git a/test/admin_test.rb b/test/admin_test.rb index bc72f93..76e029c 100644 --- a/test/admin_test.rb +++ b/test/admin_test.rb @@ -229,6 +229,40 @@ class AdminTest < Minitest::Test "sweep should remove IPs with only stale entries" end + def test_create_post_rejects_invalid_slug + login + get "/admin/posts/new" + post "/admin/posts", + "_csrf" => csrf(last_response.body), "title" => "X", "slug" => "../../etc/passwd", + "lang" => "en", "body" => "y" + assert_includes last_response.body, "Invalid slug" + end + + def test_create_post_rejects_xss_in_slug + login + get "/admin/posts/new" + post "/admin/posts", + "_csrf" => csrf(last_response.body), "title" => "X", "slug" => "foo\" onclick", + "lang" => "en", "body" => "y" + assert_includes last_response.body, "Invalid slug" + end + + def test_change_username_rejects_special_chars + login + get "/admin/settings" + post "/admin/settings/username", + "_csrf" => csrf(last_response.body), "username" => "foo\" bar" + assert_includes last_response.body, "Username may use letters" + end + + def test_change_username_accepts_same_name + login + get "/admin/settings" + post "/admin/settings/username", + "_csrf" => csrf(last_response.body), "username" => "admin" + assert_includes last_response.body, "Username unchanged" + end + private def csrf(body) diff --git a/test/store_test.rb b/test/store_test.rb index 85d1e1b..d21698c 100644 --- a/test/store_test.rb +++ b/test/store_test.rb @@ -49,4 +49,10 @@ class StoreTest < Minitest::Test assert store.find("global", lang: "cs") assert store.find("global", lang: "en") end + + def test_save_rejects_path_traversal_slug + store = Volumen::Store.new(@dir) + post = Volumen::Post.new(metadata: { "slug" => "../../etc/passwd" }, body: "x") + assert_raises(ArgumentError, "slug escapes content directory") { store.save(post) } + end end