[pdm-devel] [PATCH proxmox] rrd: restrict archive path via regex

[pdm-devel] [PATCH proxmox] rrd: restrict archive path via regex

[Lukas Wagner]

The `rel_path` parameter is used as a relative path inside the `rrdb`
base directory to build the final path for the archive file. Usually,
this is something like 'node/localhost/cpu_avg1'. For PBS, this is fine,
since these paths are hardcoded or derived from safe datastore names. In
PDM however, these paths are built from potentially 'untrusted' (as in,
one could 'pretend' to be a PBS/PVE remote and send malicious data)
metric data points - so we should have additional safe guards in place
to disallow potentially dangerous paths like '../abc' which would escape
the base directory.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 proxmox-rrd/Cargo.toml   |  4 +++-
 proxmox-rrd/src/cache.rs | 11 +++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/proxmox-rrd/Cargo.toml b/proxmox-rrd/Cargo.toml
index 5ff6ec98..240d00cc 100644
--- a/proxmox-rrd/Cargo.toml
+++ b/proxmox-rrd/Cargo.toml
@@ -16,15 +16,17 @@ proxmox-router = { workspace = true, features = ["cli", "server"] }
 [dependencies]
 anyhow.workspace = true
 bitflags.workspace = true
+const_format.workspace = true
 crossbeam-channel.workspace = true
 log.workspace = true
 nix.workspace = true
+regex.workspace = true
 serde = { workspace = true, features = ["derive"] }
 serde_cbor.workspace = true
 serde_json.workspace = true
 serde_plain.workspace = true
 
-proxmox-schema = { workspace = true, features = [ "api-macro" ] }
+proxmox-schema = { workspace = true, features = [ "api-macro", "api-types" ] }
 proxmox-sys.workspace = true
 proxmox-time.workspace = true
 
diff --git a/proxmox-rrd/src/cache.rs b/proxmox-rrd/src/cache.rs
index 29d46ed5..042b4213 100644
--- a/proxmox-rrd/src/cache.rs
+++ b/proxmox-rrd/src/cache.rs
@@ -8,8 +8,11 @@ use std::thread::spawn;
 use std::time::SystemTime;
 
 use anyhow::{bail, format_err, Error};
+use const_format::concatcp;
 use crossbeam_channel::{bounded, TryRecvError};
 
+use proxmox_schema::api_types::SAFE_ID_REGEX_STR;
+use proxmox_schema::const_regex;
 use proxmox_sys::fs::{create_path, CreateOptions};
 
 use crate::rrd::{AggregationFn, DataSourceType, Database};
@@ -21,6 +24,10 @@ use journal::*;
 mod rrd_map;
 use rrd_map::*;
 
+const_regex! {
+    DATAPOINT_PATH_REGEX = concatcp!(r"^", SAFE_ID_REGEX_STR, r"(/", SAFE_ID_REGEX_STR, r")+$");
+}
+
 /// RRD cache - keep RRD data in RAM, but write updates to disk
 ///
 /// This cache is designed to run as single instance (no concurrent
@@ -214,6 +221,10 @@ impl Cache {
         dst: DataSourceType,
         new_only: bool,
     ) -> Result<(), Error> {
+        if !DATAPOINT_PATH_REGEX.is_match(rel_path) {
+            bail!("invalid datapoint path: {rel_path}");
+        }
+
         let journal_applied = self.apply_journal()?;
 
         self.state
-- 
2.47.3



_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel