[Asterisk-code-review] res_crypto: don't modify fname in try_load_key() (asterisk[20])

[George Joseph]

George Joseph has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/19421 )

Change subject: res_crypto: don't modify fname in try_load_key()
......................................................................

res_crypto: don't modify fname in try_load_key()

"fname" is passed in as a const char *, but strstr() mangles that
into a char *, and we were attempting to modify the string in place.
This is an unwanted (and undocumented) side-effect.

ASTERISK-30213

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

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved
  Friendly Automation: Approved for Submit




diff --git a/res/res_crypto.c b/res/res_crypto.c
index 82014b6..8d6c536 100644
--- a/res/res_crypto.c
+++ b/res/res_crypto.c
@@ -174,18 +174,20 @@
 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];
+	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;
+	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;
@@ -244,8 +246,6 @@
 		}
 	}
 
-	/* Make fname just be the normal name now */
-	*c = '\0';
 	if (!key) {
 		if (!(key = ast_calloc(1, sizeof(*key)))) {
 			fclose(f);
@@ -254,8 +254,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 minus the 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/+/19421
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 20
Gerrit-Change-Id: Ifa36d352aafeb7f9beec3f746332865c7d21e629
Gerrit-Change-Number: 19421
Gerrit-PatchSet: 3
Gerrit-Owner: Philip Prindeville <philipp at redfish-solutions.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20221010/fd19221f/attachment.html>