Merge pull request 'fix reflected xss when returning errors on web endpoint' (#1472) from escape-errors-v1 into main-v1

Reviewed-on: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/1472

Cargo.lock

@@ -1679,6 +1679,7 @@ dependencies = [
  "garage_model",
  "garage_table",
  "garage_util",
+ "html-escape",
  "http 1.4.0",
  "http-body-util",
  "hyper 1.8.1",
@@ -1949,6 +1950,15 @@ dependencies = [
  "windows-sys 0.61.2",
 ]
 
+[[package]]
+name = "html-escape"
+version = "0.2.13"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "6d1ad449764d627e22bfd7cd5e8868264fc9236e07c752972b4080cd351cb476"
+dependencies = [
+ "utf8-width",
+]
+
 [[package]]
 name = "http"
 version = "0.2.12"
@@ -4820,6 +4830,12 @@ dependencies = [
  "serde",
 ]
 
+[[package]]
+name = "utf8-width"
+version = "0.1.8"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "1292c0d970b54115d14f2492fe0170adf21d68a1de108eebc51c1df4f346a091"
+
 [[package]]
 name = "utf8_iter"
 version = "1.0.4"

Cargo.toml

@@ -56,6 +56,7 @@ gethostname = "0.4"
 git-version = "0.3.4"
 hex = "0.4"
 hexdump = "0.1"
+html-escape = "0.2.13"
 hmac = "0.12"
 itertools = "0.12"
 ipnet = "2.9.0"
@@ -146,12 +147,8 @@ aws-smithy-runtime = { version = "1.8", default-features = false, features = ["t
 aws-sdk-config = { version = "1.62", default-features = false }
 aws-sdk-s3 = { version = "1.79", default-features = false, features = ["rt-tokio"] }
 
-[profile.dev]
-#lto = "thin"	# disabled for now, adds 2-4 min to each CI build
-lto = "off"
-
 [profile.release]
-lto = true
-codegen-units = 1
-opt-level = "s"
-strip = true
+lto = "thin"
+codegen-units = 16
+opt-level = 3
+strip = "debuginfo"

src/api/s3/cors.rs

@@ -88,7 +88,9 @@ pub async fn handle_put_cors(
 pub struct CorsConfiguration {
 	#[serde(serialize_with = "xmlns_tag", skip_deserializing)]
 	pub xmlns: (),
-	#[serde(rename = "CORSRule")]
+	// "default" is required to be able to parse an empty list of rules,
+	// cf https://docs.rs/quick-xml/latest/quick_xml/de/#sequences-xsall-and-xssequence-xml-schema-types
+	#[serde(rename = "CORSRule", default)]
 	pub cors_rules: Vec<CorsRule>,
 }
 
@@ -270,4 +272,26 @@ mod tests {
 
 		Ok(())
 	}
+
+	#[test]
+	fn test_deserialize_norules() -> Result<(), Error> {
+		let message = r#"<?xml version="1.0" encoding="UTF-8"?>
+<CORSConfiguration xmlns="http://s3.amazonaws.com/doc/2006-03-01/" />"#;
+		let conf: CorsConfiguration = from_str(message).unwrap();
+		let ref_value = CorsConfiguration {
+			xmlns: (),
+			cors_rules: vec![],
+		};
+		assert_eq! {
+			ref_value,
+			conf
+		};
+
+		let message2 = to_xml_with_header(&ref_value)?;
+
+		let cleanup = |c: &str| c.replace(char::is_whitespace, "");
+		assert_eq!(cleanup(message), cleanup(&message2));
+
+		Ok(())
+	}
 }

src/block/manager.rs

@@ -783,6 +783,7 @@ impl BlockManagerLocked {
 
 		let mut f = fs::File::create(&path_tmp).await?;
 		f.write_all(data).await?;
+		f.flush().await?;
 		mgr.metrics.bytes_written.add(data.len() as u64);
 
 		if mgr.data_fsync {

src/util/background/worker.rs

@@ -115,32 +115,39 @@ impl WorkerProcessor {
 						trace!("{} (TID {}): {:?}", worker.worker.name(), worker.task_id, worker.state);
 
 						// Save worker info
-						let mut wi = self.worker_info.lock().unwrap();
-						match wi.get_mut(&worker.task_id) {
-							Some(i) => {
-								i.state = worker.state;
-								i.status = worker.worker.status();
-								i.errors = worker.errors;
-								i.consecutive_errors = worker.consecutive_errors;
-								if worker.last_error.is_some() {
-									i.last_error = worker.last_error.take();
+						{
+							let mut wi = self.worker_info.lock().unwrap();
+							match wi.get_mut(&worker.task_id) {
+								Some(i) => {
+									i.state = worker.state;
+									i.status = worker.worker.status();
+									i.errors = worker.errors;
+									i.consecutive_errors = worker.consecutive_errors;
+									if worker.last_error.is_some() {
+										i.last_error = worker.last_error.take();
+									}
+								}
+								None => {
+									wi.insert(worker.task_id, WorkerInfo {
+										name: worker.worker.name(),
+										state: worker.state,
+										status: worker.worker.status(),
+										errors: worker.errors,
+										consecutive_errors: worker.consecutive_errors,
+										last_error: worker.last_error.take(),
+									});
 								}
-							}
-							None => {
-								wi.insert(worker.task_id, WorkerInfo {
-									name: worker.worker.name(),
-									state: worker.state,
-									status: worker.worker.status(),
-									errors: worker.errors,
-									consecutive_errors: worker.consecutive_errors,
-									last_error: worker.last_error.take(),
-								});
 							}
 						}
 
 						if worker.state == WorkerState::Done {
 							info!("Worker {} (TID {}) exited", worker.worker.name(), worker.task_id);
 						} else {
+							// Yield to the Tokio scheduler between consecutive Busy steps so
+							// that a worker which never suspends on its own cannot starve other tasks.
+							if worker.state == WorkerState::Busy {
+								tokio::task::yield_now().await;
+							}
 							workers.push(async move {
 								worker.step().await;
 								worker

src/web/Cargo.toml

@@ -20,6 +20,7 @@ garage_model.workspace = true
 garage_util.workspace = true
 garage_table.workspace = true
 
+html-escape.workspace = true
 thiserror.workspace = true
 tracing.workspace = true
 percent-encoding.workspace = true

src/web/web_server.rs

@@ -411,7 +411,7 @@ fn error_to_res(e: Error) -> Response<BoxBody<Error>> {
 <li>Message: {s3_message}.</li>
 </ul>",
 			s3_code = err.aws_code(),
-			s3_message = err,
+			s3_message = html_escape::encode_text(&err.to_string()),
 		));
 	}
 	let mut http_error = Response::new(string_body(body_str));