From a1a0262e304d71b57a5648c1631782131d31b75c Mon Sep 17 00:00:00 2001 From: Pavan Kumar Reddy B Date: Mon, 15 Jun 2026 21:20:10 +0530 Subject: [PATCH 01/12] added a strlen guard before the memcmp to avoid reading past the end of a short string --- source/jst_functions.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/jst_functions.c b/source/jst_functions.c index a09e7ad..4edd0bf 100644 --- a/source/jst_functions.c +++ b/source/jst_functions.c @@ -615,7 +615,7 @@ static duk_ret_t do_openssl_verify_with_cert(duk_context *ctx) /* === NOW PROCEED WITH SIGNATURE VERIFICATION === */ //open certificate file - if(memcmp(filepath, "file://", sizeof("file://")-1) != 0) + if(strlen(filepath) < (sizeof("file://") - 1) || memcmp(filepath, "file://", sizeof("file://")-1) != 0) { CosaPhpExtLog("openssl_verify_with_cert: file %s doesn't begin with 'file://'\n", filepath); free(sig_bytes); From 2d4ad29f9d03f6383afd780fd05504b6715f3de8 Mon Sep 17 00:00:00 2001 From: Pavan Kumar Reddy B Date: Mon, 15 Jun 2026 21:42:21 +0530 Subject: [PATCH 02/12] Check `ftell` and `fseek`'s return values --- source/jst_internal.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/source/jst_internal.c b/source/jst_internal.c index e7312f0..5edfee1 100644 --- a/source/jst_internal.c +++ b/source/jst_internal.c @@ -179,8 +179,22 @@ int read_file(const char *filename, char** bufout, size_t* lenout) return 0; } - fseek(pf, 0, SEEK_END); - size = ftell(pf); + if(fseek(pf, 0, SEEK_END) != 0) + { + fclose(pf); + fprintf(stderr, "Error: fseek failed %s\n", filename); + return 0; + } + { + long ftell_result = ftell(pf); + if(ftell_result < 0) + { + fclose(pf); + fprintf(stderr, "Error: ftell failed %s (not a regular file?)\n", filename); + return 0; + } + size = (size_t)ftell_result; + } rewind(pf); buf = (char*)calloc(size+1, 1); From bb8b5811307b0f8c33237febb36c024dc2af357e Mon Sep 17 00:00:00 2001 From: Pavan Kumar Reddy B Date: Mon, 15 Jun 2026 21:59:30 +0530 Subject: [PATCH 03/12] Added allocation failure check for `session_id`. Ensured `session_id` is freed on **all** exit paths (including success). Allocated the new identifier into a temporary pointer (`new_session_identifier`) first. Freed old `session_identifier` (if present) before replacing it. Assigned `session_identifier = new_session_identifier` only after successful construction. --- source/jst_session.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/source/jst_session.c b/source/jst_session.c index ceec720..a038d9b 100644 --- a/source/jst_session.c +++ b/source/jst_session.c @@ -162,8 +162,15 @@ static duk_ret_t session_create(duk_context *ctx) int i = 0, n = 0; uint8_t bytes[SESSION_ID_BYTES_LENGTH]; char* session_id = NULL; + char* new_session_identifier = NULL; session_id = (char*)malloc(SESSION_ID_BYTES_LENGTH+1); + if(!session_id) + { + CosaPhpExtLog("Failed to allocate session_id!\n"); + RETURN_FALSE; + } + n = syscall(SYS_getrandom, bytes, SESSION_ID_BYTES_LENGTH, 0); if(n != SESSION_ID_BYTES_LENGTH) { @@ -177,16 +184,25 @@ static duk_ret_t session_create(duk_context *ctx) session_id[i] = BYTE_TO_PRINTABLE_HEX_CODE(bytes[i]); } - session_identifier = (char*)malloc(SESSION_ID_LENGTH+1); - if(!session_identifier) + new_session_identifier = (char*)malloc(SESSION_ID_LENGTH+1); + if(!new_session_identifier) { CosaPhpExtLog("Failed to allocate session_identifier!\n"); + free(session_id); RETURN_FALSE; } - memset(session_identifier, 0, SESSION_ID_LENGTH+1); + memset(new_session_identifier, 0, SESSION_ID_LENGTH+1); session_id[SESSION_ID_BYTES_LENGTH] = '\0'; - snprintf(session_identifier, SESSION_ID_LENGTH+1, "%s%s", SESSION_PREFIX, session_id); + snprintf(new_session_identifier, SESSION_ID_LENGTH+1, "%s%s", SESSION_PREFIX, session_id); + + free(session_id); + + if(session_identifier) + { + free(session_identifier); + } + session_identifier = new_session_identifier; RETURN_TRUE; return 1; From da27d8aec3e0d9d72ff86f66fd3514aba970a4c9 Mon Sep 17 00:00:00 2001 From: Pavan Kumar Reddy B Date: Mon, 15 Jun 2026 22:26:43 +0530 Subject: [PATCH 04/12] In `do_exec()`, removed shell-based execution and replaced it with a safe process flow. No shell is used to execute user input. Shell metacharacter injection (`;`, `&&`, `|`, etc.) no longer gets interpreted as command chaining. Output is still captured line-by-line and returned as the same JS array structure. --- source/jst_functions.c | 67 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 60 insertions(+), 7 deletions(-) diff --git a/source/jst_functions.c b/source/jst_functions.c index 4edd0bf..7136d7a 100644 --- a/source/jst_functions.c +++ b/source/jst_functions.c @@ -21,6 +21,8 @@ #include #include #include +#include +#include #include "jst_internal.h" #include "jst.h" @@ -153,6 +155,12 @@ static duk_ret_t do_exec(duk_context *ctx) ssize_t nread; duk_idx_t idx; int index = 0; + wordexp_t args; + int wr; + int pipes[2] = { -1, -1 }; + pid_t pid; + FILE* pipe_stream = NULL; + int status = 0; idx = duk_push_array(ctx); @@ -161,15 +169,54 @@ static duk_ret_t do_exec(duk_context *ctx) CosaPhpExtLog("exec command=%s\n", command); - FILE* pipe = popen(command, "r"); - if (!pipe) + wr = wordexp(command, &args, WRDE_NOCMD); + if (wr != 0 || args.we_wordc == 0) { - CosaPhpExtLog("exec failed to open pipe\n"); - duk_pop(ctx); - return 1; + CosaPhpExtLog("exec failed to parse command, wordexp status=%d\n", wr); + return 1; + } + + if (pipe(pipes) != 0) + { + CosaPhpExtLog("exec failed to create pipe, error:%s\n", strerror(errno)); + wordfree(&args); + return 1; } - while((nread = getline(&line, &len, pipe)) != -1) + pid = fork(); + if (pid < 0) + { + CosaPhpExtLog("exec fork failed, error:%s\n", strerror(errno)); + close(pipes[0]); + close(pipes[1]); + wordfree(&args); + return 1; + } + + if (pid == 0) + { + close(pipes[0]); + if (dup2(pipes[1], STDOUT_FILENO) < 0) + { + _exit(127); + } + close(pipes[1]); + execvp(args.we_wordv[0], args.we_wordv); + _exit(127); + } + + close(pipes[1]); + pipe_stream = fdopen(pipes[0], "r"); + if (!pipe_stream) + { + CosaPhpExtLog("exec failed to open read pipe, error:%s\n", strerror(errno)); + close(pipes[0]); + waitpid(pid, &status, 0); + wordfree(&args); + return 1; + } + + while((nread = getline(&line, &len, pipe_stream)) != -1) { CosaPhpExtLog("exec line: %s\n", line); duk_push_string(ctx, line); @@ -177,7 +224,13 @@ static duk_ret_t do_exec(duk_context *ctx) } free(line); - pclose(pipe); + fclose(pipe_stream); + waitpid(pid, &status, 0); + if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) + { + CosaPhpExtLog("exec command exited abnormally, status=%d\n", status); + } + wordfree(&args); return 1; } From e0114543d38d266f525ba82861d68960874992df Mon Sep 17 00:00:00 2001 From: Pavan Kumar Reddy B Date: Mon, 15 Jun 2026 22:34:23 +0530 Subject: [PATCH 05/12] Removed unsafe call `strtok()` and replaced with non-mutating parsing. This avoids modifying memory returned from `getenv()`. --- source/jst_session.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/source/jst_session.c b/source/jst_session.c index a038d9b..cea24c2 100644 --- a/source/jst_session.c +++ b/source/jst_session.c @@ -128,14 +128,25 @@ static duk_ret_t session_start(duk_context *ctx) } if(isvalid) { - sesid = strtok(sesid, ";"); + char sesid_token[SESSION_ID_LENGTH+1]; + size_t sesid_token_len = strcspn(sesid, ";"); + + if (sesid_token_len < SESSION_ID_LENGTH) + { + CosaPhpExtLog("Invalid SessionID Entropy\n"); + RETURN_FALSE; + } + + memcpy(sesid_token, sesid, SESSION_ID_LENGTH); + sesid_token[SESSION_ID_LENGTH] = '\0'; + const char filename[SESSION_FILE_MAX_PATH]; - snprintf(filename, SESSION_FILE_MAX_PATH, "%s/%s", SESSION_TMP_DIR, sesid); + snprintf(filename, SESSION_FILE_MAX_PATH, "%s/%s", SESSION_TMP_DIR, sesid_token); CosaPhpExtLog("%s: Checking for Session file %s\n", __PRETTY_FUNCTION__, filename); if (access(filename, F_OK) == 0) { CosaPhpExtLog("%s: Session file %s exists\n", __PRETTY_FUNCTION__, filename); - strncpy(session_identifier, sesid, SESSION_ID_LENGTH); + strncpy(session_identifier, sesid_token, SESSION_ID_LENGTH); } else { CosaPhpExtLog("%s: Failed to read Session file %s\n", __PRETTY_FUNCTION__, filename); } From 030d6b0fd8952c988264a5729d47f37dcc7de409 Mon Sep 17 00:00:00 2001 From: Pavan Kumar Reddy B Date: Mon, 15 Jun 2026 22:38:39 +0530 Subject: [PATCH 06/12] Tightened session-cookie validation in `session_start()` to prevent bypass/hijack paths: - Require cookie token length to be **exactly** `SESSION_ID_LENGTH` (not just `>=`). - Copy the token safely into a local buffer and validate there. - Enforce required prefix: token must start with `SESSION_PREFIX` (`"jst_sess"`). - Validate the random suffix chars with `isalnum` on `unsigned char`. - Only if all checks pass, check for the corresponding session file and load `session_identifier`. --- source/jst_session.c | 80 +++++++++++++++++++++++--------------------- 1 file changed, 42 insertions(+), 38 deletions(-) diff --git a/source/jst_session.c b/source/jst_session.c index cea24c2..19dfb52 100644 --- a/source/jst_session.c +++ b/source/jst_session.c @@ -112,45 +112,49 @@ static duk_ret_t session_start(duk_context *ctx) if(sesid) { sesid += 7; - int len = strlen(sesid); - if(len >= SESSION_ID_LENGTH) + size_t sesid_token_len = strcspn(sesid, ";"); + if(sesid_token_len == SESSION_ID_LENGTH) { - int idx = SESSION_PREFIX_LEN; - int isvalid = 1; - /* Validate session ID*/ - while ( idx < SESSION_ID_LENGTH) { - if (!isalnum(sesid[idx])) { - CosaPhpExtLog("Invalid SessionID\n"); - isvalid = 0; - break; - } - idx++; - } - if(isvalid) - { - char sesid_token[SESSION_ID_LENGTH+1]; - size_t sesid_token_len = strcspn(sesid, ";"); - - if (sesid_token_len < SESSION_ID_LENGTH) - { - CosaPhpExtLog("Invalid SessionID Entropy\n"); - RETURN_FALSE; - } - - memcpy(sesid_token, sesid, SESSION_ID_LENGTH); - sesid_token[SESSION_ID_LENGTH] = '\0'; - - const char filename[SESSION_FILE_MAX_PATH]; - snprintf(filename, SESSION_FILE_MAX_PATH, "%s/%s", SESSION_TMP_DIR, sesid_token); - CosaPhpExtLog("%s: Checking for Session file %s\n", __PRETTY_FUNCTION__, filename); - if (access(filename, F_OK) == 0) - { - CosaPhpExtLog("%s: Session file %s exists\n", __PRETTY_FUNCTION__, filename); - strncpy(session_identifier, sesid_token, SESSION_ID_LENGTH); - } else { - CosaPhpExtLog("%s: Failed to read Session file %s\n", __PRETTY_FUNCTION__, filename); - } - } + int idx = SESSION_PREFIX_LEN; + int isvalid = 1; + char sesid_token[SESSION_ID_LENGTH+1]; + + memcpy(sesid_token, sesid, SESSION_ID_LENGTH); + sesid_token[SESSION_ID_LENGTH] = '\0'; + + if(strncmp(sesid_token, SESSION_PREFIX, SESSION_PREFIX_LEN) != 0) + { + CosaPhpExtLog("Invalid SessionID prefix\n"); + isvalid = 0; + } + + /* Validate random portion of session ID */ + while (idx < SESSION_ID_LENGTH && isvalid) + { + if(!isalnum((unsigned char)sesid_token[idx])) + { + CosaPhpExtLog("Invalid SessionID\n"); + isvalid = 0; + break; + } + idx++; + } + + if(isvalid) + { + char filename[SESSION_FILE_MAX_PATH]; + snprintf(filename, SESSION_FILE_MAX_PATH, "%s/%s", SESSION_TMP_DIR, sesid_token); + CosaPhpExtLog("%s: Checking for Session file %s\n", __PRETTY_FUNCTION__, filename); + if (access(filename, F_OK) == 0) + { + CosaPhpExtLog("%s: Session file %s exists\n", __PRETTY_FUNCTION__, filename); + strncpy(session_identifier, sesid_token, SESSION_ID_LENGTH); + } + else + { + CosaPhpExtLog("%s: Failed to read Session file %s\n", __PRETTY_FUNCTION__, filename); + } + } } else { CosaPhpExtLog("Invalid SessionID Entropy\n"); } From 621952c0b2cdd3713b0f5521d148b59bdbdaa66f Mon Sep 17 00:00:00 2001 From: Pavan Kumar Reddy B Date: Mon, 15 Jun 2026 22:44:54 +0530 Subject: [PATCH 07/12] Copilot Suggestion Optimize file:// prefix validation: replace strlen+memcmp with fixed-length strncmp --- source/jst_functions.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/jst_functions.c b/source/jst_functions.c index 7136d7a..0323b38 100644 --- a/source/jst_functions.c +++ b/source/jst_functions.c @@ -668,7 +668,7 @@ static duk_ret_t do_openssl_verify_with_cert(duk_context *ctx) /* === NOW PROCEED WITH SIGNATURE VERIFICATION === */ //open certificate file - if(strlen(filepath) < (sizeof("file://") - 1) || memcmp(filepath, "file://", sizeof("file://")-1) != 0) + if(strncmp(filepath, "file://", sizeof("file://") - 1) != 0) { CosaPhpExtLog("openssl_verify_with_cert: file %s doesn't begin with 'file://'\n", filepath); free(sig_bytes); From 939fce11c4439b8f537186722c5db5573b7e25fa Mon Sep 17 00:00:00 2001 From: Pavan Kumar Reddy B Date: Mon, 15 Jun 2026 22:48:06 +0530 Subject: [PATCH 08/12] Copilot Suggestion Harden read_file size handling: validate ftell range and guard size+1 overflow --- source/jst_internal.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/source/jst_internal.c b/source/jst_internal.c index 5edfee1..25b54c4 100644 --- a/source/jst_internal.c +++ b/source/jst_internal.c @@ -193,10 +193,25 @@ int read_file(const char *filename, char** bufout, size_t* lenout) fprintf(stderr, "Error: ftell failed %s (not a regular file?)\n", filename); return 0; } + + if((unsigned long)ftell_result > (unsigned long)((size_t)-1)) + { + fclose(pf); + fprintf(stderr, "Error: file too large to represent safely %s\n", filename); + return 0; + } + size = (size_t)ftell_result; } rewind(pf); + if(size > ((size_t)-1) - 1) + { + fclose(pf); + fprintf(stderr, "Error: file size overflow %s\n", filename); + return 0; + } + buf = (char*)calloc(size+1, 1); if(!buf) { From 77af3c070ada361151437a534ec32cd5460a6540 Mon Sep 17 00:00:00 2001 From: Pavan Kumar Reddy B Date: Tue, 16 Jun 2026 13:17:54 +0530 Subject: [PATCH 09/12] Resource leak fix from Code Scanning - `args` is now properly freed with `wordfree(&args)` --- source/jst_functions.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/source/jst_functions.c b/source/jst_functions.c index 0323b38..9246eb8 100644 --- a/source/jst_functions.c +++ b/source/jst_functions.c @@ -170,11 +170,17 @@ static duk_ret_t do_exec(duk_context *ctx) CosaPhpExtLog("exec command=%s\n", command); wr = wordexp(command, &args, WRDE_NOCMD); - if (wr != 0 || args.we_wordc == 0) + if (wr != 0) { CosaPhpExtLog("exec failed to parse command, wordexp status=%d\n", wr); return 1; } + if (args.we_wordc == 0) + { + CosaPhpExtLog("exec empty command after expansion\n"); + wordfree(&args); + return 1; + } if (pipe(pipes) != 0) { From ca149088cd87e1dc34231ea418aff22b33f5ea6d Mon Sep 17 00:00:00 2001 From: Pavan Kumar Reddy B Date: Tue, 16 Jun 2026 13:23:16 +0530 Subject: [PATCH 10/12] Check the `waitpid` return, loop on `EINTR`, and log exit-code/signal clearly. --- source/jst_functions.c | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/source/jst_functions.c b/source/jst_functions.c index 9246eb8..d35e4c3 100644 --- a/source/jst_functions.c +++ b/source/jst_functions.c @@ -217,7 +217,12 @@ static duk_ret_t do_exec(duk_context *ctx) { CosaPhpExtLog("exec failed to open read pipe, error:%s\n", strerror(errno)); close(pipes[0]); - waitpid(pid, &status, 0); + { + pid_t wp; + do { wp = waitpid(pid, &status, 0); } while (wp == -1 && errno == EINTR); + if (wp == -1) + CosaPhpExtLog("exec waitpid failed, error:%s\n", strerror(errno)); + } wordfree(&args); return 1; } @@ -231,10 +236,27 @@ static duk_ret_t do_exec(duk_context *ctx) free(line); fclose(pipe_stream); - waitpid(pid, &status, 0); - if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) { - CosaPhpExtLog("exec command exited abnormally, status=%d\n", status); + pid_t wp; + do { wp = waitpid(pid, &status, 0); } while (wp == -1 && errno == EINTR); + if (wp == -1) + { + CosaPhpExtLog("exec waitpid failed, error:%s\n", strerror(errno)); + } + else if (WIFEXITED(status)) + { + int exit_code = WEXITSTATUS(status); + if (exit_code != 0) + CosaPhpExtLog("exec command exited with code %d\n", exit_code); + } + else if (WIFSIGNALED(status)) + { + CosaPhpExtLog("exec command killed by signal %d\n", WTERMSIG(status)); + } + else + { + CosaPhpExtLog("exec command exited abnormally, raw status=%d\n", status); + } } wordfree(&args); From 1fb15a66130c321ea02de03521b4efc024623416 Mon Sep 17 00:00:00 2001 From: Pavan Kumar Reddy B Date: Tue, 16 Jun 2026 13:25:47 +0530 Subject: [PATCH 11/12] Fix always-false ftell range check: use SIZE_MAX under preprocessor guard --- source/jst_internal.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/source/jst_internal.c b/source/jst_internal.c index 25b54c4..b86a531 100644 --- a/source/jst_internal.c +++ b/source/jst_internal.c @@ -19,6 +19,7 @@ #include "jst_internal.h" #include #include +#include #include #include @@ -194,12 +195,14 @@ int read_file(const char *filename, char** bufout, size_t* lenout) return 0; } - if((unsigned long)ftell_result > (unsigned long)((size_t)-1)) +#if LONG_MAX > SIZE_MAX + if((unsigned long)ftell_result > (unsigned long)SIZE_MAX) { fclose(pf); fprintf(stderr, "Error: file too large to represent safely %s\n", filename); return 0; } +#endif size = (size_t)ftell_result; } From ea14e954e537d00200c863ee5f47b4e884da5ecd Mon Sep 17 00:00:00 2001 From: Pavan Kumar Reddy B Date: Tue, 16 Jun 2026 13:27:44 +0530 Subject: [PATCH 12/12] Explicitly NUL-terminate session_identifier after strncpy --- source/jst_session.c | 1 + 1 file changed, 1 insertion(+) diff --git a/source/jst_session.c b/source/jst_session.c index 19dfb52..a39c60b 100644 --- a/source/jst_session.c +++ b/source/jst_session.c @@ -149,6 +149,7 @@ static duk_ret_t session_start(duk_context *ctx) { CosaPhpExtLog("%s: Session file %s exists\n", __PRETTY_FUNCTION__, filename); strncpy(session_identifier, sesid_token, SESSION_ID_LENGTH); + session_identifier[SESSION_ID_LENGTH] = '\0'; } else {