fix /report endpoint a lot

in short, the `/report` endpoint now checks if:
- the reporting room in the URI matches the PDU/event reported
- sender user is in the room reported
- raises report reasoning to 750 characters (spec doesn't say to limit
these, but thorough and informative reports for server admins are not
a bad thing)
- (hopefully) fixes some broken formatting
- add a random short delay before sending a successful response to the
client to make it more annoying to enumerate for events on our server
(security by obscurity but spec suggests it)

basically, secure reports better lol

see https://spec.matrix.org/v1.9/client-server-api/#post_matrixclientv3roomsroomidreporteventid

Signed-off-by: strawberry <strawberry@puppygock.gay>

DIFFERENCES.md

@@ -52,3 +52,4 @@
 - Only follow 6 redirects total in our default reqwest ClientBuilder
 - Generate passwords with 25 characters instead of 15
 - Add missing `reason` field to user ban events (`/ban`)
+- For all [`/report`](https://spec.matrix.org/v1.9/client-server-api/#post_matrixclientv3roomsroomidreporteventid) requests: check if the reported event ID belongs to the reported room ID, raise report reasoning character limit to 750, fix broken formatting, make a small delayed random response per spec suggestion on privacy, and check if the sender user is in the reported room.

src/api/client_server/report.rs

@@ -1,9 +1,14 @@
+use std::time::Duration;
+
 use crate::{services, utils::HtmlEscape, Error, Result, Ruma};
+use rand::Rng;
 use ruma::{
     api::client::{error::ErrorKind, room::report_content},
     events::room::message,
     int,
 };
+use tokio::time::sleep;
+use tracing::{debug, info};
 
 /// # `POST /_matrix/client/v3/rooms/{roomId}/report/{eventId}`
 ///
@@ -12,18 +17,30 @@ use ruma::{
 pub async fn report_event_route(
     body: Ruma<report_content::v3::Request>,
 ) -> Result<report_content::v3::Response> {
+    // user authentication
     let sender_user = body.sender_user.as_ref().expect("user is authenticated");
 
+    info!("Received /report request by user {}", sender_user);
+
+    // check if we know about the reported event ID or if it's invalid
     let pdu = match services().rooms.timeline.get_pdu(&body.event_id)? {
         Some(pdu) => pdu,
         _ => {
             return Err(Error::BadRequest(
-                ErrorKind::InvalidParam,
-                "Invalid Event ID",
+                ErrorKind::NotFound,
+                "Event ID is not known to us or Event ID is invalid",
             ))
         }
     };
 
+    // check if the room ID from the URI matches the PDU's room ID
+    if body.room_id != pdu.room_id {
+        return Err(Error::BadRequest(
+            ErrorKind::NotFound,
+            "Event ID does not belong to the reported room",
+        ));
+    }
+
     // check if reporting user is in the reporting room
     if !services()
         .rooms
@@ -38,6 +55,7 @@ pub async fn report_event_route(
         ));
     }
 
+    // check if score is in valid range
     if let Some(true) = body.score.map(|s| s > int!(0) || s < int!(-100)) {
         return Err(Error::BadRequest(
             ErrorKind::InvalidParam,
@@ -45,13 +63,15 @@ pub async fn report_event_route(
         ));
     };
 
-    if let Some(true) = body.reason.clone().map(|s| s.chars().count() > 500) {
+    // check if report reasoning is less than or equal to 750 characters
+    if let Some(true) = body.reason.clone().map(|s| s.chars().count() >= 750) {
         return Err(Error::BadRequest(
             ErrorKind::InvalidParam,
-            "Reason too long, should be 500 characters or fewer",
+            "Reason too long, should be 750 characters or fewer",
         ));
     };
 
+    // send admin room message that we received the report with an @room ping for urgency
     services().admin
         .send_message(message::RoomMessageEventContent::text_html(
             format!(
@@ -59,18 +79,18 @@ pub async fn report_event_route(
                 Event ID: {:?}\n\
                 Room ID: {:?}\n\
                 Sent By: {:?}\n\n\
-                Report Score: {:?}\n\
+                Report Score: {:#?}\n\
                 Report Reason: {:?}",
-                sender_user, pdu.event_id, pdu.room_id, pdu.sender, body.score, body.reason
+                sender_user.to_owned(), pdu.event_id, pdu.room_id, pdu.sender, body.score, body.reason
             ),
             format!(
-                "<details><summary>Report received from: <a href=\"https://matrix.to/#/{0:?}\">{0:?}\
+                "<details><summary>@room Report received from: <a href=\"https://matrix.to/#/{0}\">{0}\
                 </a></summary><ul><li>Event Info<ul><li>Event ID: <code>{1:?}</code>\
                 <a href=\"https://matrix.to/#/{2:?}/{1:?}\">🔗</a></li><li>Room ID: <code>{2:?}</code>\
                 </li><li>Sent By: <a href=\"https://matrix.to/#/{3:?}\">{3:?}</a></li></ul></li><li>\
                 Report Info<ul><li>Report Score: {4:?}</li><li>Report Reason: {5}</li></ul></li>\
                 </ul></details>",
-                sender_user,
+                sender_user.to_owned(),
                 pdu.event_id,
                 pdu.room_id,
                 pdu.sender,
@@ -79,5 +99,14 @@ pub async fn report_event_route(
             ),
         ));
 
+    // even though this is kinda security by obscurity, let's still make a small random delay sending a successful response
+    // per spec suggestion regarding enumerating for potential events existing in our server.
+    let time_to_wait = rand::thread_rng().gen_range(8..21);
+    debug!(
+        "Got successful /report request, waiting {} seconds before sending successful response.",
+        time_to_wait
+    );
+    sleep(Duration::from_secs(time_to_wait)).await;
+
     Ok(report_content::v3::Response {})
 }