Security: Validate blog controller inputs (page param, blog ID)
Ran a claude /security-review, fixed two vulnerabilities Use a plug to resolve blog_id, returning a clean 404 for unknown blogs instead of raising with inspect(). Parse page param with Integer.parse so invalid values (non-numeric, negative, zero) fall back to page 1 instead of crashing. Add 5 tests covering these cases.a
This commit is contained in:
parent
c8bc3ea197
commit
6887ae4087
@ -1,9 +1,11 @@
|
|||||||
defmodule FirehoseWeb.BlogController do
|
defmodule FirehoseWeb.BlogController do
|
||||||
use FirehoseWeb, :controller
|
use FirehoseWeb, :controller
|
||||||
|
|
||||||
def index(conn, %{"blog_id" => blog_id} = params) do
|
plug :resolve_blog
|
||||||
blog = resolve_blog!(blog_id)
|
|
||||||
page = String.to_integer(params["page"] || "1")
|
def index(conn, params) do
|
||||||
|
blog = conn.assigns.blog
|
||||||
|
page = parse_page(params["page"])
|
||||||
result = blog.paginate(page)
|
result = blog.paginate(page)
|
||||||
|
|
||||||
render(conn, :index,
|
render(conn, :index,
|
||||||
@ -17,8 +19,8 @@ defmodule FirehoseWeb.BlogController do
|
|||||||
)
|
)
|
||||||
end
|
end
|
||||||
|
|
||||||
def show(conn, %{"blog_id" => blog_id, "slug" => slug}) do
|
def show(conn, %{"slug" => slug}) do
|
||||||
blog = resolve_blog!(blog_id)
|
blog = conn.assigns.blog
|
||||||
post = blog.get_post!(slug)
|
post = blog.get_post!(slug)
|
||||||
|
|
||||||
render(conn, :show,
|
render(conn, :show,
|
||||||
@ -28,8 +30,8 @@ defmodule FirehoseWeb.BlogController do
|
|||||||
)
|
)
|
||||||
end
|
end
|
||||||
|
|
||||||
def tag(conn, %{"blog_id" => blog_id, "tag" => tag}) do
|
def tag(conn, %{"tag" => tag}) do
|
||||||
blog = resolve_blog!(blog_id)
|
blog = conn.assigns.blog
|
||||||
posts = blog.posts_by_tag(tag)
|
posts = blog.posts_by_tag(tag)
|
||||||
|
|
||||||
render(conn, :tag,
|
render(conn, :tag,
|
||||||
@ -41,10 +43,25 @@ defmodule FirehoseWeb.BlogController do
|
|||||||
)
|
)
|
||||||
end
|
end
|
||||||
|
|
||||||
defp resolve_blog!("engineering"), do: Firehose.EngineeringBlog
|
defp resolve_blog(%{params: %{"blog_id" => "engineering"}} = conn, _opts),
|
||||||
defp resolve_blog!("releases"), do: Firehose.ReleaseNotes
|
do: assign(conn, :blog, Firehose.EngineeringBlog)
|
||||||
|
|
||||||
defp resolve_blog!(id) do
|
defp resolve_blog(%{params: %{"blog_id" => "releases"}} = conn, _opts),
|
||||||
raise Blogex.NotFoundError, "unknown blog: #{inspect(id)}"
|
do: assign(conn, :blog, Firehose.ReleaseNotes)
|
||||||
|
|
||||||
|
defp resolve_blog(conn, _opts) do
|
||||||
|
conn
|
||||||
|
|> put_status(:not_found)
|
||||||
|
|> put_view(FirehoseWeb.ErrorHTML)
|
||||||
|
|> render(:"404")
|
||||||
|
|> halt()
|
||||||
|
end
|
||||||
|
|
||||||
|
defp parse_page(nil), do: 1
|
||||||
|
defp parse_page(str) do
|
||||||
|
case Integer.parse(str) do
|
||||||
|
{page, ""} when page > 0 -> page
|
||||||
|
_ -> 1
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@ -26,6 +26,34 @@ defmodule FirehoseWeb.BlogTest do
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe "input validation" do
|
||||||
|
test "GET /blog/nonexistent returns 404", %{conn: conn} do
|
||||||
|
conn = get(conn, "/blog/nonexistent")
|
||||||
|
assert html_response(conn, 404)
|
||||||
|
end
|
||||||
|
|
||||||
|
test "GET /blog/engineering?page=abc falls back to page 1", %{conn: conn} do
|
||||||
|
conn = get(conn, "/blog/engineering?page=abc")
|
||||||
|
assert html_response(conn, 200) =~ "Engineering Blog"
|
||||||
|
end
|
||||||
|
|
||||||
|
test "GET /blog/engineering?page=-1 falls back to page 1", %{conn: conn} do
|
||||||
|
conn = get(conn, "/blog/engineering?page=-1")
|
||||||
|
assert html_response(conn, 200) =~ "Engineering Blog"
|
||||||
|
end
|
||||||
|
|
||||||
|
test "GET /blog/engineering?page=0 falls back to page 1", %{conn: conn} do
|
||||||
|
conn = get(conn, "/blog/engineering?page=0")
|
||||||
|
assert html_response(conn, 200) =~ "Engineering Blog"
|
||||||
|
end
|
||||||
|
|
||||||
|
test "GET /blog/engineering/nonexistent-post returns 404", %{conn: conn} do
|
||||||
|
assert_raise Blogex.NotFoundError, fn ->
|
||||||
|
get(conn, "/blog/engineering/nonexistent-post")
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
describe "release notes blog (HTML)" do
|
describe "release notes blog (HTML)" do
|
||||||
test "GET /blog/releases returns HTML index", %{conn: conn} do
|
test "GET /blog/releases returns HTML index", %{conn: conn} do
|
||||||
conn = get(conn, "/blog/releases")
|
conn = get(conn, "/blog/releases")
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user