Update README: When to use raou, clarifications

Fix the most embarassing kind of path traversal vulnerability
imaginable for such a tool.

You could simply run raou ../../../../tmp/evil_entry

The C version contained various check on the config dir and its
entries which would have prevented this attack. In this port,
the checking functions were deemed unnecessary, as they
did lots of redundant checks too. Unfortunately, I missed this
trivial attack when I decided not to port them.

At the plus side, I found this now myself while sleep-deprived, so
there may be some hope for me after all.

Also, you should not use some non-released software from some
guys git ;-)

README.md

@@ -7,9 +7,16 @@ specified by the administrator.
 
 Originally written in C, it's now reimplemented in Rust.
 
-By default, raou looks in  /etc/raou.d/ for config files. If you run
-"raou backup", it will look for /etc/raou.d/backup.
+### When to use raou (over sudo) 
+Generally, it's not a replacement for sudo. The primary use case of raou is a situation in which you would want to allow a user to run a privileged operation as root without entering passwords. You may not want to use sudo for that, particularly if you don't have it installed already. Some further arguments for raou:
+   
+   - Simpler config
+   - Less complexity, less attack surface
+   - Writte in a memory-safe language
 
+### Config
+By default, raou looks in  ```/etc/raou.d/``` for config files. If you run
+"raou backup", it will look for ```/etc/raou.d/backup```.
 Example config file:
 ```
 user john
@@ -20,29 +27,29 @@ path /usr/local/bin/script.sh
 **user** is the name of the user who you want to give permissions to 
 execute **path** as the **target_user**.  
 
-**path** must contain the absolute path. 
+**path** must contain the absolute path of the to be executed command. 
 
-Optional fields
----------------
-**args**: If you want to leave out optional arguments (argv) to *path*, 
-simply don't  include this. Otherwise, simply specify them
+#### Optional fields
+
+**args** (string): If you want to leave out optional arguments (argv) to *path*, 
+simply don't  include this. Otherwise, specify them here.
 ```
 ...
 args -v -ltr 
 ```
-**allow_args**: Allow arbitrary arguments, so:
+**allow_args** (1 or 0, default 0): Allow arbitrary arguments, so:
 ```
 raou backup /path
 ```
 
-Will launch "path" as specified in the file for the backup entry with "/path" as argv[1] instead of the argument specified with "args" in the config file.
+Will execute the command specified in **path** of the  ```backup``` entry with "/path" as argv[1] instead of the argument specified with "args" in the config file.
 
-**no_new_privs**: Defaults to 1. Processes launched with this option active
-won't be able to gain more privileges, even when they call setuid programs.
+**no_new_privs** (1 or 0, default 1): Processes launched with this option active
+won't be able to gain more privileges, even when they call setuid programs. This can break some programs.
 
-**env_vars**: A comma-separated list of environment variables to inherit
+**env_vars** (string): A comma-separated list of environment variables to inherit
 from the current environment. Everything else will be wiped (but others
 like HOME, SHELL etc. will be appropriately set). 
 
-**argv0**: Set this option if you want to provide your own value as "argv0"
+**argv0** (string): Set this option if you want to provide your own value as "argv0"
 The default is the name of the launched binary (not the whole path). 

src/main.rs

@@ -5,6 +5,7 @@ use std::fs::File;
 use std::io::BufRead;
 use std::io::BufReader;
 use std::io::{Error, ErrorKind};
+use std::fs;
 
 extern crate libc;
 use libc::passwd;
@@ -47,7 +48,9 @@ struct Passwd {
 
 fn initgroups(user: &str, group: libc::gid_t) -> std::io::Result<()> {
     let userarg = CString::new(user);
-    return errnowrapper(unsafe { libc::initgroups(userarg.unwrap().as_ptr(), group) });
+    return errnowrapper(unsafe {
+        libc::initgroups(userarg.unwrap().as_ptr(), group)
+    });
 }
 
 fn errnowrapper(ret: libc::c_int) -> std::io::Result<()> {
@@ -75,12 +78,25 @@ fn getpwnam(username: &str) -> std::io::Result<Passwd> {
     fn getstr(str: *mut libc::c_char) -> String {
         unsafe { CStr::from_ptr(str).to_string_lossy().into_owned() }
     }
-        let username_c = CString::new(username).unwrap();
-        let username_ptr = username_c.as_ptr();
-        let pwnamresult: *mut libc::passwd = unsafe { libc::getpwnam(username_ptr) };
-        if pwnamresult.is_null() {
-            return Err(Error::new(Error::last_os_error().kind(),"Lookup of user failed: ".to_owned() + &Error::last_os_error().to_string()));
+    let username_c = CString::new(username).unwrap();
+    let username_ptr = username_c.as_ptr();
+    let pwnamresult: *mut libc::passwd = unsafe { libc::getpwnam(username_ptr) };
+    if pwnamresult.is_null() {
+        if Error::last_os_error().raw_os_error().unwrap() == 0 {
+            return Err(Error::new(
+                ErrorKind::NotFound,
+                format!("The username '{}' was not found", username),
+            ));
         }
+        return Err(Error::new(
+            Error::last_os_error().kind(),
+            format!(
+                "Lookup of user {} failed: {}",
+                username,
+                &Error::last_os_error().to_string()
+            ),
+        ));
+    }
     unsafe {
         Ok(Passwd {
             pw_name: getstr((*pwnamresult).pw_name),
@@ -172,8 +188,7 @@ fn clearenv() -> std::io::Result<()> {
 }
 //TODO: AsRef for envs?
 fn setup_environment(passwd: &Passwd, envs: &[String]) -> std::io::Result<()> {
-    let saved_envs: Vec<String> = envs
-        .iter()
+    let saved_envs: Vec<String> = envs.iter()
         .map(|s| std::env::var(s).expect("No such var"))
         .collect();
     clearenv()?;
@@ -201,7 +216,9 @@ fn become_user(passwd: &Passwd) -> std::io::Result<()> {
 fn drop_privs(entry: &Entry) -> std::io::Result<()> {
     if entry.no_new_privs {
         errnowrapper(unsafe { libc::prctl(libc::PR_SET_DUMPABLE, 0) })?;
-        errnowrapper(unsafe { libc::prctl(libc::PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) })?;
+        errnowrapper(unsafe {
+            libc::prctl(libc::PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)
+        })?;
     }
     Ok(())
 }
@@ -233,14 +250,35 @@ fn create_execv_args(entry: &Entry, cmdargs: &Vec<String>) -> Vec<*const libc::c
     return args;
 }
 fn exec(entryname: &str, cmdargs: &Vec<String>) -> std::io::Result<()> {
-    let mut filepath: String = String::from("/etc/raou.d/");
-    filepath = filepath + entryname;
+    let basedir: String = String::from("/etc/raou.d/");
+    let filepath: String = basedir.to_string() + entryname;
 
-    if !std::path::Path::new(&filepath).exists() {
-        return Err(std::io::Error::new(
-            ErrorKind::NotFound,
-            "The entry ".to_owned() + &filepath + " does not exist",
-        ));
+    let realpath = fs::canonicalize(&filepath);
+    match realpath {
+        Ok(p) => {
+            if !p.starts_with(basedir) {
+                return Err(std::io::Error::new(
+                    ErrorKind::InvalidInput,
+                    "Specified entry is outside base directory",
+                ));
+            }
+            if !p.is_file() {
+                return Err(std::io::Error::new(
+                    ErrorKind::Other,
+                    "Error: Entry not a file",
+                ));
+            }
+        }
+        Err(e) => {
+            if e.kind() == ErrorKind::NotFound {
+                return Err(std::io::Error::new(
+                    ErrorKind::NotFound,
+                    format!("The entry {} does not exist", entryname),
+                ));
+            } else {
+                return Err(e);
+            }
+        }
     }
     let entry: Entry = create_entry_from_file(&filepath)?;
     let destuserpasswd: Passwd = getpwnam(&entry.dest_user)?;
@@ -252,30 +290,32 @@ fn exec(entryname: &str, cmdargs: &Vec<String>) -> std::io::Result<()> {
     become_user(&destuserpasswd).or_else(|e| {
         return Err(Error::new(
             ErrorKind::PermissionDenied,
-            "Failed to switch user: ".to_owned() + &e.to_string(),
-        ));
-    })?;
-    setup_environment(&destuserpasswd, &entry.inherit_envs).or_else(|e| {
-        return Err(Error::new(
-            ErrorKind::Other,
-            "Environment setup failure: ".to_owned() + &e.to_string(),
+            format!("Failed to switch user: {}", &e.to_string()),
         ));
     })?;
+    setup_environment(&destuserpasswd, &entry.inherit_envs)
+        .or_else(|e| {
+            return Err(Error::new(
+                ErrorKind::Other,
+                format!("Environment setup failure: {}", &e.to_string()),
+            ));
+        })?;
 
     drop_privs(&entry).or_else(|e| {
         return Err(Error::new(
             ErrorKind::Other,
-            "Failed to drop priviliges: ".to_owned() + &e.to_string(),
+            format!("Failed to drop priviliges: {}", &e.to_string()),
         ));
     })?;
 
     unsafe {
-        errnowrapper(libc::execv(to_cstring(entry.cmd), args.as_ptr())).or_else(|e| {
-            return Err(Error::new(
-                ErrorKind::Other,
-                "execv failed: ".to_owned() + &e.to_string(),
-            ));
-        })?;
+        errnowrapper(libc::execv(to_cstring(entry.cmd), args.as_ptr()))
+            .or_else(|e| {
+                return Err(Error::new(
+                    ErrorKind::Other,
+                    format!("execv failed: {}", &e.to_string()),
+                ));
+            })?;
     }
     std::process::exit(0);
 }
@@ -284,7 +324,12 @@ fn main() -> Result<(), std::io::Error> {
     let cmdargs: Vec<String> = argv.collect();
     let entryname = cmdargs.get(1);
     if entryname.is_some() {
-        match exec(&entryname.unwrap(), &cmdargs) {
+        let entry = entryname.unwrap();
+        if !entry.chars().all(|c| c.is_alphanumeric() || c == '.') {
+            eprintln!("Entry names can only contain alphanumeric characters and dots");
+            std::process::exit(1);
+        }
+        match exec(entry, &cmdargs) {
             Err(e) => {
                 eprintln!("The following error ocurred:");
                 eprintln!("{}", e);