[pbs-devel] [PATCH proxmox v3] fix #3302: allow for more characters for email

[pbs-devel] [PATCH proxmox v3] fix #3302: allow for more characters for email

[Dominik Csapak]

by removing the regex check here, that is responsibility of the caller

this is ok since we pass the args directly and not via shell, so
command injection should not be possible

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
tested command injection with emails like '--help' but this got sent
to '--help@myhostname' which got sent to 'root@myhostname'

 proxmox/src/tools/email.rs | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/proxmox/src/tools/email.rs b/proxmox/src/tools/email.rs
index b5d42c4..0b92a5b 100644
--- a/proxmox/src/tools/email.rs
+++ b/proxmox/src/tools/email.rs
@@ -16,23 +16,10 @@ pub fn sendmail(
     mailfrom: Option<&str>,
     author: Option<&str>,
 ) -> Result<(), Error> {
-    let mail_regex = regex::Regex::new(r"^[a-zA-Z\.0-9-]+@[a-zA-Z\.0-9-]+$").unwrap();
-
     if mailto.is_empty() {
         bail!("At least one recipient has to be specified!")
     }
-
-    for recipient in mailto {
-        if !mail_regex.is_match(recipient) {
-            bail!("'{}' is not a valid email address", recipient)
-        }
-    }
-
     let mailfrom = mailfrom.unwrap_or("root");
-    if !mailfrom.eq("root") && !mail_regex.is_match(mailfrom) {
-        bail!("'{}' is not a valid email address", mailfrom)
-    }
-
     let recipients = mailto.join(",");
     let author = author.unwrap_or("Proxmox Backup Server");
 
@@ -44,7 +31,7 @@ pub fn sendmail(
         .arg("-f")
         .arg(mailfrom)
         .arg("--")
-        .arg(&recipients)
+        .args(mailto)
         .stdin(Stdio::piped())
         .spawn()
     {
-- 
2.20.1

[pbs-devel] applied: [PATCH proxmox v3] fix #3302: allow for more characters for email

[Thomas Lamprecht]

On 12.05.21 16:20, Dominik Csapak wrote:
> by removing the regex check here, that is responsibility of the caller
> 
> this is ok since we pass the args directly and not via shell, so
> command injection should not be possible

yeah, if nothing is there to interpret injected commands then it really isn't
possible, besides naturally some security issue in sendmail command parser or
the like, but there's no future proofing against that.. ;-)

> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> tested command injection with emails like '--help' but this got sent
> to '--help@myhostname' which got sent to 'root@myhostname'
> 
>  proxmox/src/tools/email.rs | 15 +--------------
>  1 file changed, 1 insertion(+), 14 deletions(-)
> 
>

applied, thanks!