Merge pull request #577 from ppp-project/strict

Strict access checks for scripts, secrets files and call option files.

Author: paulusmack

pppd/auth.c

@@ -1545,7 +1545,8 @@ check_passwd(int unit,
      * Open the file of pap secrets and scan for a suitable secret
      * for authenticating this user.
      */
-    filename = path_upapfile;
+    if (!ppp_check_access(path_upapfile, &filename, 0, 0))
+	    return UPAP_AUTHNAK;
     addrs = opts = NULL;
     ret = UPAP_AUTHNAK;
     f = fopen(filename, "r");
@@ -1587,6 +1588,7 @@ check_passwd(int unit,
 	}
 	fclose(f);
     }
+    free(filename);
 
     if (ret == UPAP_AUTHNAK) {
         if (**msg == 0)
@@ -1646,17 +1648,21 @@ null_login(int unit)
      * Open the file of pap secrets and scan for a suitable secret.
      */
     if (ret <= 0) {
-	filename = path_upapfile;
+	if (!ppp_check_access(path_upapfile, &filename, 0, 0))
+	    return 0;
 	addrs = NULL;
 	f = fopen(filename, "r");
-	if (f == NULL)
+	if (f == NULL) {
+	    free(filename);
 	    return 0;
+	}
 	check_access(f, filename);
 
 	i = scan_authfile(f, "", our_name, secret, &addrs, &opts, filename, 0);
 	ret = i >= 0 && secret[0] == 0;
 	BZERO(secret, sizeof(secret));
 	fclose(f);
+	free(filename);
     }
 
     if (ret)
@@ -1730,14 +1736,18 @@ have_pap_secret(int *lacks_ipp)
 	    return ret;
     }
 
-    filename = path_upapfile;
+    if (!ppp_check_access(path_upapfile, &filename, 0, 0))
+	return 0;
     f = fopen(filename, "r");
-    if (f == NULL)
+    if (f == NULL) {
+	free(filename);
 	return 0;
+    }
 
     ret = scan_authfile(f, (explicit_remote? remote_name: NULL), our_name,
 			NULL, &addrs, NULL, filename, 0);
     fclose(f);
+    free(filename);
     if (ret >= 0 && !some_ip_ok(addrs)) {
 	if (lacks_ipp != 0)
 	    *lacks_ipp = 1;
@@ -1772,10 +1782,13 @@ have_chap_secret(char *client, char *server,
 	}
     }
 
-    filename = path_chapfile;
+    if (!ppp_check_access(path_chapfile, &filename, 0, 0))
+	    return 0;
     f = fopen(filename, "r");
-    if (f == NULL)
+    if (f == NULL) {
+	free(filename);
 	return 0;
+    }
 
     if (client != NULL && client[0] == 0)
 	client = NULL;
@@ -1784,6 +1797,7 @@ have_chap_secret(char *client, char *server,
 
     ret = scan_authfile(f, client, server, NULL, &addrs, NULL, filename, 0);
     fclose(f);
+    free(filename);
     if (ret >= 0 && need_ip && !some_ip_ok(addrs)) {
 	if (lacks_ipp != 0)
 	    *lacks_ipp = 1;
@@ -1810,10 +1824,13 @@ have_srp_secret(char *client, char *server, int need_ip, int *lacks_ipp)
     char *filename;
     struct wordlist *addrs;
 
-    filename = PPP_PATH_SRPFILE;
+    if (!ppp_check_access(PPP_PATH_SRPFILE, &filename, 0, 0))
+	return 0;
     f = fopen(filename, "r");
-    if (f == NULL)
+    if (f == NULL) {
+	free(filename);
 	return 0;
+    }
 
     if (client != NULL && client[0] == 0)
 	client = NULL;
@@ -1822,6 +1839,7 @@ have_srp_secret(char *client, char *server, int need_ip, int *lacks_ipp)
 
     ret = scan_authfile(f, client, server, NULL, &addrs, NULL, filename, 0);
     fclose(f);
+    free(filename);
     if (ret >= 0 && need_ip && !some_ip_ok(addrs)) {
 	if (lacks_ipp != 0)
 	    *lacks_ipp = 1;
@@ -1858,19 +1876,22 @@ get_secret(int unit, char *client, char *server,
 	    return 0;
 	}
     } else {
-	filename = path_chapfile;
+	if (!ppp_check_access(path_chapfile, &filename, 0, 0))
+	    return 0;
 	addrs = NULL;
 	secbuf[0] = 0;
 
 	f = fopen(filename, "r");
 	if (f == NULL) {
 	    error("Can't open chap secret file %s: %m", filename);
+	    free(filename);
 	    return 0;
 	}
 	check_access(f, filename);
 
 	ret = scan_authfile(f, client, server, secbuf, &addrs, &opts, filename, 0);
 	fclose(f);
+	free(filename);
 	if (ret < 0)
 	    return 0;
 
@@ -1912,12 +1933,14 @@ get_srp_secret(int unit, char *client, char *server,
     if (!am_server && passwd[0] != '\0') {
 	strlcpy(secret, passwd, MAXWORDLEN);
     } else {
-	filename = PPP_PATH_SRPFILE;
+	if (!ppp_check_access(PPP_PATH_SRPFILE, &filename, 0, 0))
+	    return 0;
 	addrs = NULL;
 
 	fp = fopen(filename, "r");
 	if (fp == NULL) {
 	    error("Can't open srp secret file %s: %m", filename);
+	    free(filename);
 	    return 0;
 	}
 	check_access(fp, filename);
@@ -1926,6 +1949,7 @@ get_srp_secret(int unit, char *client, char *server,
 	ret = scan_authfile(fp, client, server, secret, &addrs, &opts,
 	    filename, am_server);
 	fclose(fp);
+	free(filename);
 	if (ret < 0)
 	    return 0;
 
@@ -2304,18 +2328,25 @@ scan_authfile(FILE *f, char *client, char *server,
 	     * Special syntax: @/pathname means read secret from file.
 	     */
 	    if (word[0] == '@' && word[1] == '/') {
+		char *realname;
+
 		strlcpy(atfile, word+1, sizeof(atfile));
-		if ((sf = fopen(atfile, "r")) == NULL) {
+		if (!ppp_check_access(atfile, &realname, 0, 0))
+		    continue;
+		if ((sf = fopen(realname, "r")) == NULL) {
 		    warn("can't open indirect secret file %s", atfile);
+		    free(realname);
 		    continue;
 		}
 		check_access(sf, atfile);
 		if (!getword(sf, word, &xxx, atfile)) {
 		    warn("no secret in indirect secret file %s", atfile);
 		    fclose(sf);
+		    free(realname);
 		    continue;
 		}
 		fclose(sf);
+		free(realname);
 	    }
 	    strlcpy(lsecret, word, sizeof(lsecret));
 	}
@@ -2474,10 +2505,13 @@ have_eaptls_secret_server(char *client, char *server,
     char cacertfile[MAXWORDLEN];
     char pkfile[MAXWORDLEN];
 
-    filename = PPP_PATH_EAPTLSSERVFILE;
+    if (!ppp_check_access(PPP_PATH_EAPTLSSERVFILE, &filename, 0, 0))
+	return 0;
     f = fopen(filename, "r");
-    if (f == NULL)
-		return 0;
+    if (f == NULL) {
+	free(filename);
+	return 0;
+    }
 
     if (client != NULL && client[0] == 0)
 		client = NULL;
@@ -2490,6 +2524,7 @@ have_eaptls_secret_server(char *client, char *server,
 			     0);
 
     fclose(f);
+    free(filename);
 
 /*
     if (ret >= 0 && !eaptls_init_ssl(1, cacertfile, servcertfile,
@@ -2752,10 +2787,13 @@ get_eaptls_secret(int unit, char *client, char *server,
 		filename = (am_server ? PPP_PATH_EAPTLSSERVFILE : PPP_PATH_EAPTLSCLIFILE);
 		addrs = NULL;
 
+		if (!ppp_check_access(filename, &filename, 0, 0))
+			return 0;
 		fp = fopen(filename, "r");
 		if (fp == NULL)
 		{
 			error("Can't open eap-tls secret file %s: %m", filename);
+			free(filename);
 			return 0;
 		}
 
@@ -2765,6 +2803,7 @@ get_eaptls_secret(int unit, char *client, char *server,
 				cacertfile, pkfile, &addrs, &opts, filename, 0);
 
 		fclose(fp);
+		free(filename);
 
 		if (ret < 0) return 0;
 	}

pppd/main.c

@@ -1921,21 +1921,14 @@ pid_t
 run_program(char *prog, char * const *args, int must_exist, void (*done)(void *), void *arg, int wait)
 {
     int pid, status, ret;
-    struct stat sbuf;
+    char *rpath;
 
     /*
-     * First check if the file exists and is executable.
-     * We don't use access() because that would use the
-     * real user-id, which might not be root, and the script
-     * might be accessible only to root.
+     * First check if the file exists and is executable by root,
+     * and couldn't have been modified by a non-root process.
      */
-    errno = EINVAL;
-    if (stat(prog, &sbuf) < 0 || !S_ISREG(sbuf.st_mode)
-	|| (sbuf.st_mode & (S_IXUSR|S_IXGRP|S_IXOTH)) == 0) {
-	if (must_exist || errno != ENOENT)
-	    warn("Can't execute %s: %m", prog);
+    if (!ppp_check_access(prog, &rpath, must_exist, 1))
 	return 0;
-    }
 
     pid = ppp_safe_fork(fd_devnull, fd_devnull, fd_devnull);
     if (pid == -1) {
@@ -1954,6 +1947,7 @@ run_program(char *prog, char * const *args, int must_exist, void (*done)(void *)
 	    }
 	    forget_child(pid, status);
 	}
+	free(rpath);
 	return pid;
     }
 
@@ -1981,12 +1975,12 @@ run_program(char *prog, char * const *args, int must_exist, void (*done)(void *)
 
     /* run the program */
     update_script_environment();
-    execve(prog, args, script_env);
+    execve(rpath, args, script_env);
     if (must_exist || errno != ENOENT) {
 	/* have to reopen the log, there's nowhere else
 	   for the message to go. */
 	reopen_log();
-	syslog(LOG_ERR, "Can't execute %s: %m", prog);
+	syslog(LOG_ERR, "Can't execute %s: %m", rpath);
 	closelog();
     }
     _exit(99);

pppd/options.c

@@ -1640,6 +1640,7 @@ callfile(char **argv)
 {
     char *fname, *arg, *p;
     int l, ok;
+    char *realname;
 
     arg = *argv;
     ok = 1;
@@ -1668,9 +1669,15 @@ callfile(char **argv)
     slprintf(fname, l, "%s%s", PPP_PATH_PEERFILES, arg);
     ppp_script_setenv("CALL_FILE", arg, 0);
 
-    ok = ppp_options_from_file(fname, 1, 1, 1);
+    if (!ppp_check_access(fname, &realname, 1, 0)) {
+	free(fname);
+	return 0;
+    }
+
+    ok = ppp_options_from_file(realname, 1, 1, 1);
 
     free(fname);
+    free(realname);
     return ok;
 }
 

pppd/pppd.h

@@ -299,6 +299,9 @@ void pr_log(void *, char *, ...);
 /* finish up after using pr_log */
 void end_pr_log(void);
 
+/* Check that a file can safely be used */
+int ppp_check_access(const char *path, char **path_to_use, int must_exist, int exec);
+
 /*
  * Get the current exist status of pppd
  */