From a398abb2fe57ee51eb2cde293223c868f25b1a30 Mon Sep 17 00:00:00 2001
From: "Rob Swindell (on Debian Linux)" <rob@synchro.net>
Date: Sat, 14 Sep 2024 14:12:43 -0700
Subject: [PATCH] Don't heap allocate argument to MsgBase and FileBase
 constructors

Nelgin reported a weird error with a failed very large allocation for the
base/code argument to the FileBase constructor. There's no good reason
these strings were heap-allocated in the first place, so just change to
use a stack allocated variable instead. I don't know why this would fix
anything, but at least there's one less heap allocation and potential
for memory leak here.

Fix 2 bugs in js_update_file():
1. missing parenthesis (really?!? Caught by Coverity - sigh) in last
commmit caused attempt/failure/error to remove file after making any
updates. The removing is only supposed to happen when its necessary to
remove and re-add the file to the filebase (e.g. updating extended
description or auxdata).
2. Wrong filename used in 'removing' exception string.
---
 src/sbbs3/js_filebase.c | 18 +++++++-----------
 src/sbbs3/js_msgbase.c  | 14 ++++----------
 2 files changed, 11 insertions(+), 21 deletions(-)

diff --git a/src/sbbs3/js_filebase.c b/src/sbbs3/js_filebase.c
index 1bf74cc00b..af362dedc0 100644
--- a/src/sbbs3/js_filebase.c
+++ b/src/sbbs3/js_filebase.c
@@ -1330,18 +1330,18 @@ js_update_file(JSContext *cx, uintN argc, jsval *arglist)
 				if(file.extdesc != NULL)
 					truncsp(file.extdesc);
 				if(!readd_always && strcmp(extdesc ? extdesc : "", file.extdesc ? file.extdesc : "") == 0
-					&& strcmp(auxdata ? auxdata : "", file.auxdata ? file.auxdata : "") == 0)
+					&& strcmp(auxdata ? auxdata : "", file.auxdata ? file.auxdata : "") == 0) {
 					p->smb_result = smb_putfile(&p->smb, &file);
 					if(p->smb_result != SMB_SUCCESS)
 						JS_ReportError(cx, "%d writing '%s'", p->smb_result, file.name);
-				else {
+				} else {
 					if((p->smb_result = smb_removefile_by_name(&p->smb, filename)) == SMB_SUCCESS) {
 						if(readd_always)
 							file.hdr.when_imported.time = 0; // we want the file to appear as "new"
 						p->smb_result = smb_addfile(&p->smb, &file, SMB_SELFPACK, extdesc, auxdata, newfname);
 					}
 					else
-						JS_ReportError(cx, "%d removing '%s'", p->smb_result, file.name);
+						JS_ReportError(cx, "%d removing '%s'", p->smb_result, filename);
 				}
 			}
 		}
@@ -1811,7 +1811,7 @@ js_filebase_constructor(JSContext *cx, uintN argc, jsval *arglist)
 {
 	JSObject *		obj;
 	jsval *			argv=JS_ARGV(cx, arglist);
-	char*			base = NULL;
+	char			base[MAX_PATH + 1] = "";
 	private_t*		p;
 	scfg_t*			scfg;
 
@@ -1827,15 +1827,13 @@ js_filebase_constructor(JSContext *cx, uintN argc, jsval *arglist)
 	memset(p,0,sizeof(private_t));
 	p->smb.retry_time=scfg->smb_retry_time;
 
-	JSSTRING_TO_MSTRING(cx, JS_ValueToString(cx, argv[0]), base, NULL);
+	JSVALUE_TO_STRBUF(cx, argv[0], base, sizeof base, NULL);
 	if(JS_IsExceptionPending(cx)) {
-		if(base != NULL)
-			free(base);
 		free(p);
 		return JS_FALSE;
 	}
-	if(base==NULL) {
-		JS_ReportError(cx, "Invalid base parameter");
+	if(*base=='\0') {
+		JS_ReportError(cx, "Invalid 'code' parameter");
 		free(p);
 		return JS_FALSE;
 	}
@@ -1843,7 +1841,6 @@ js_filebase_constructor(JSContext *cx, uintN argc, jsval *arglist)
 	if(!JS_SetPrivate(cx, obj, p)) {
 		JS_ReportError(cx,"JS_SetPrivate failed");
 		free(p);
-		free(base);
 		return JS_FALSE;
 	}
 
@@ -1864,7 +1861,6 @@ js_filebase_constructor(JSContext *cx, uintN argc, jsval *arglist)
 		SAFECOPY(p->smb.file, base);
 	}
 
-	free(base);
 	return JS_TRUE;
 }
 
diff --git a/src/sbbs3/js_msgbase.c b/src/sbbs3/js_msgbase.c
index eef2374c6b..7f6c1be5d8 100644
--- a/src/sbbs3/js_msgbase.c
+++ b/src/sbbs3/js_msgbase.c
@@ -3350,9 +3350,8 @@ js_msgbase_constructor(JSContext *cx, uintN argc, jsval *arglist)
 {
 	JSObject *		obj;
 	jsval *			argv=JS_ARGV(cx, arglist);
-	JSString*		js_str;
 	JSObject*		cfgobj;
-	char*			base = NULL;
+	char			base[MAX_PATH + 1] = "";
 	private_t*		p;
 	scfg_t*			scfg;
 
@@ -3368,16 +3367,13 @@ js_msgbase_constructor(JSContext *cx, uintN argc, jsval *arglist)
 	memset(p,0,sizeof(private_t));
 	p->smb.retry_time=scfg->smb_retry_time;
 
-	js_str = JS_ValueToString(cx, argv[0]);
-	JSSTRING_TO_MSTRING(cx, js_str, base, NULL);
+	JSVALUE_TO_STRBUF(cx, argv[0], base, sizeof base, NULL);
 	if(JS_IsExceptionPending(cx)) {
-		if(base != NULL)
-			free(base);
 		free(p);
 		return JS_FALSE;
 	}
-	if(base==NULL) {
-		JS_ReportError(cx, "Invalid base parameter");
+	if(*base=='\0') {
+		JS_ReportError(cx, "Invalid 'code' parameter");
 		free(p);
 		return JS_FALSE;
 	}
@@ -3387,7 +3383,6 @@ js_msgbase_constructor(JSContext *cx, uintN argc, jsval *arglist)
 	if(!JS_SetPrivate(cx, obj, p)) {
 		JS_ReportError(cx,"JS_SetPrivate failed");
 		free(p);
-		free(base);
 		return JS_FALSE;
 	}
 
@@ -3441,7 +3436,6 @@ js_msgbase_constructor(JSContext *cx, uintN argc, jsval *arglist)
 		}
 	}
 
-	free(base);
 	return JS_TRUE;
 }
 
-- 
GitLab