[Asterisk-code-review] res_crypto: handle unsafe private key files (asterisk[master])

[Philip Prindeville]

Philip Prindeville has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/19278 )


Change subject: res_crypto: handle unsafe private key files
......................................................................

res_crypto: handle unsafe private key files

Note that strstr() takes a "const char *" but returns a "char *"
which we're then scribbling on.  That's a no-no.  We should
preserve the argument sent to us in try_load_key() as "fname".

ASTERISK-30213 #close

Change-Id: I4a77143d41615b7c4fc25bb1251c0a9cb87b417a
---
M res/res_crypto.c
1 file changed, 46 insertions(+), 7 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/78/19278/1

diff --git a/res/res_crypto.c b/res/res_crypto.c
index 82014b6..41daa8d 100644
--- a/res/res_crypto.c
+++ b/res/res_crypto.c
@@ -34,6 +34,7 @@
 #include "asterisk.h"
 
 #include <dirent.h>                 /* for closedir, opendir, readdir, DIR */
+#include <sys/stat.h>               /* for fstat */
 
 #include <openssl/err.h>            /* for ERR_print_errors_fp */
 #include <openssl/ssl.h>            /* for NID_sha1, RSA */
@@ -173,19 +174,22 @@
 */
 static struct ast_key *try_load_key(const char *dir, const char *fname, int ifd, int ofd, int *not2)
 {
-	int ktype = 0, found = 0;
-	char *c = NULL, ffname[256];
+	int n, ktype = 0, found = 0;
+	const char *c = NULL;
+	char ffname[256];
 	unsigned char digest[MD5_DIGEST_LENGTH];
 	unsigned digestlen;
 	FILE *f;
 	EVP_MD_CTX *ctx = NULL;
 	struct ast_key *key;
 	static int notice = 0;
+	struct stat st;
+	size_t fnamelen = strlen(fname);
 
 	/* Make sure its name is a public or private key */
-	if ((c = strstr(fname, ".pub")) && !strcmp(c, ".pub")) {
+	if (fnamelen > 4 && !strcmp((c = &fname[fnamelen - 4]), ".pub")) {
 		ktype = AST_KEY_PUBLIC;
-	} else if ((c = strstr(fname, ".key")) && !strcmp(c, ".key")) {
+	} else if (fnamelen > 4 && !strcmp((c = &fname[fnamelen - 4]), ".key")) {
 		ktype = AST_KEY_PRIVATE;
 	} else {
 		return NULL;
@@ -200,6 +204,27 @@
 		return NULL;
 	}
 
+	n = fstat(fileno(f), &st);
+	if (n != 0) {
+		ast_log(LOG_ERROR, "Unable to stat key file: %s: %s\n", ffname, strerror(errno));
+		fclose(f);
+		return NULL;
+	}
+
+	if (!S_ISREG(st.st_mode)) {
+		ast_log(LOG_ERROR, "Key file is not a regular file: %s\n", ffname);
+		fclose(f);
+		return NULL;
+	}
+
+	/* only user read or read/write modes allowed */
+	if (ktype == AST_KEY_PRIVATE &&
+	    ((st.st_mode & ALLPERMS) & ~(S_IRUSR | S_IWUSR)) != 0) {
+		ast_log(LOG_ERROR, "Private key file has bad permissions: %s: %#4o\n", ffname, st.st_mode & ALLPERMS);
+		fclose(f);
+		return NULL;
+	}
+
 	ctx = EVP_MD_CTX_create();
 	if (ctx == NULL) {
 		ast_log(LOG_ERROR, "Out of memory\n");
@@ -245,7 +270,6 @@
 	}
 
 	/* Make fname just be the normal name now */
-	*c = '\0';
 	if (!key) {
 		if (!(key = ast_calloc(1, sizeof(*key)))) {
 			fclose(f);
@@ -254,8 +278,8 @@
 	}
 	/* First the filename */
 	ast_copy_string(key->fn, ffname, sizeof(key->fn));
-	/* Then the name */
-	ast_copy_string(key->name, fname, sizeof(key->name));
+	/* Then the name less suffix */
+	snprintf(key->name, sizeof(key->name), "%.*s", (int)(c - fname), fname);
 	key->ktype = ktype;
 	/* Yes, assume we're going to be deleted */
 	key->delme = 1;

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/19278
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I4a77143d41615b7c4fc25bb1251c0a9cb87b417a
Gerrit-Change-Number: 19278
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Prindeville <philipp at redfish-solutions.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220916/a38c5a61/attachment.html>