مراية لـ
https://github.com/postalserver/postal.git
تم المزامنة 2026-06-03 21:45:48 +00:00
The Legacy API message lookup endpoints parsed the request body as JSON and passed the `id` parameter straight through to the message database. A JSON object supplied for `id` arrived as a Ruby Hash and was used as a raw set of SQL `WHERE` conditions. `hash_to_sql` interpolated each Hash key directly inside backtick identifier quoting while escaping only the value, so a key containing a backtick could break out of the identifier and inject arbitrary SQL into the SELECT (blind, time-based) against the message database. Fixes: - Escape all identifiers (columns, tables, database names) through a new `escape_identifier` helper that wraps in backticks and doubles embedded backticks. Applied across hash_to_sql, select, insert, insert_multi, update and delete so no caller can inject via an identifier. - Validate the Legacy API `id` parameter at the controller boundary: reject any non-scalar value before it reaches the database and coerce it to an integer. Internal Hash-based lookups (e.g. tracking middleware) are unaffected. Adds regression tests at the unit (hash_to_sql / escape_identifier) and request (legacy messages/deliveries) levels.
136 أسطر
6.1 KiB
Ruby
136 أسطر
6.1 KiB
Ruby
# frozen_string_literal: true
|
|
|
|
require "rails_helper"
|
|
|
|
RSpec.describe "Legacy Messages API", type: :request do
|
|
describe "/api/v1/messages/deliveries" do
|
|
context "when no authentication is provided" do
|
|
it "returns an error" do
|
|
post "/api/v1/messages/deliveries"
|
|
expect(response.status).to eq 200
|
|
parsed_body = JSON.parse(response.body)
|
|
expect(parsed_body["status"]).to eq "error"
|
|
expect(parsed_body["data"]["code"]).to eq "AccessDenied"
|
|
end
|
|
end
|
|
|
|
context "when the credential does not match anything" do
|
|
it "returns an error" do
|
|
post "/api/v1/messages/deliveries", headers: { "x-server-api-key" => "invalid" }
|
|
expect(response.status).to eq 200
|
|
parsed_body = JSON.parse(response.body)
|
|
expect(parsed_body["status"]).to eq "error"
|
|
expect(parsed_body["data"]["code"]).to eq "InvalidServerAPIKey"
|
|
end
|
|
end
|
|
|
|
context "when the credential belongs to a suspended server" do
|
|
it "returns an error" do
|
|
server = create(:server, :suspended)
|
|
credential = create(:credential, server: server)
|
|
post "/api/v1/messages/deliveries", headers: { "x-server-api-key" => credential.key }
|
|
expect(response.status).to eq 200
|
|
parsed_body = JSON.parse(response.body)
|
|
expect(parsed_body["status"]).to eq "error"
|
|
expect(parsed_body["data"]["code"]).to eq "ServerSuspended"
|
|
end
|
|
end
|
|
|
|
context "when the credential is valid" do
|
|
let(:server) { create(:server) }
|
|
let(:credential) { create(:credential, server: server) }
|
|
|
|
context "when no message ID is provided" do
|
|
it "returns an error" do
|
|
post "/api/v1/messages/deliveries", headers: { "x-server-api-key" => credential.key }
|
|
expect(response.status).to eq 200
|
|
parsed_body = JSON.parse(response.body)
|
|
expect(parsed_body["status"]).to eq "parameter-error"
|
|
expect(parsed_body["data"]["message"]).to match(/`id` parameter is required but is missing/)
|
|
end
|
|
end
|
|
|
|
context "when the message ID does not exist" do
|
|
it "returns an error" do
|
|
post "/api/v1/messages/deliveries",
|
|
headers: { "x-server-api-key" => credential.key,
|
|
"content-type" => "application/json" },
|
|
params: { id: 123 }.to_json
|
|
expect(response.status).to eq 200
|
|
parsed_body = JSON.parse(response.body)
|
|
expect(parsed_body["status"]).to eq "error"
|
|
expect(parsed_body["data"]["code"]).to eq "MessageNotFound"
|
|
expect(parsed_body["data"]["id"]).to eq 123
|
|
end
|
|
end
|
|
|
|
# Regression test for GHSA-x2hq-rfpg-3xr5 (see message_spec.rb). A JSON
|
|
# object supplied for `id` must be rejected before reaching the database
|
|
# rather than being interpreted as a raw set of SQL conditions.
|
|
context "when the message ID is a JSON object (SQL injection attempt)" do
|
|
it "rejects it with a parameter error and never reaches the database" do
|
|
expect_any_instance_of(Server).not_to receive(:message)
|
|
post "/api/v1/messages/deliveries",
|
|
headers: { "x-server-api-key" => credential.key,
|
|
"content-type" => "application/json" },
|
|
params: { id: { "id`=0 OR SLEEP(5)#" => "x" } }.to_json
|
|
expect(response.status).to eq 200
|
|
parsed_body = JSON.parse(response.body)
|
|
expect(parsed_body["status"]).to eq "parameter-error"
|
|
expect(parsed_body["data"]["message"]).to match(/must be a string or integer/)
|
|
end
|
|
end
|
|
|
|
context "when the message ID exists" do
|
|
let(:server) { create(:server) }
|
|
let(:credential) { create(:credential, server: server) }
|
|
let(:message) { MessageFactory.outgoing(server) }
|
|
|
|
before do
|
|
message.create_delivery("SoftFail", details: "no server found",
|
|
output: "404",
|
|
sent_with_ssl: true,
|
|
log_id: "1234",
|
|
time: 1.2)
|
|
message.create_delivery("Sent", details: "sent successfully",
|
|
output: "200",
|
|
sent_with_ssl: false,
|
|
log_id: "5678",
|
|
time: 2.2)
|
|
end
|
|
|
|
before do
|
|
post "/api/v1/messages/deliveries",
|
|
headers: { "x-server-api-key" => credential.key,
|
|
"content-type" => "application/json" },
|
|
params: { id: message.id }.to_json
|
|
end
|
|
|
|
it "returns an array of deliveries" do
|
|
expect(response.status).to eq 200
|
|
parsed_body = JSON.parse(response.body)
|
|
expect(parsed_body["status"]).to eq "success"
|
|
expect(parsed_body["data"]).to match([
|
|
{ "id" => kind_of(Integer),
|
|
"status" => "SoftFail",
|
|
"details" => "no server found",
|
|
"output" => "404",
|
|
"sent_with_ssl" => true,
|
|
"log_id" => "1234",
|
|
"time" => 1.2,
|
|
"timestamp" => kind_of(Float) },
|
|
{ "id" => kind_of(Integer),
|
|
"status" => "Sent",
|
|
"details" => "sent successfully",
|
|
"output" => "200",
|
|
"sent_with_ssl" => false,
|
|
"log_id" => "5678",
|
|
"time" => 2.2,
|
|
"timestamp" => kind_of(Float) },
|
|
])
|
|
end
|
|
end
|
|
end
|
|
end
|
|
end
|