From 5c3ebc573c69c1d0eb1a61061b2bda21744aaa07 Mon Sep 17 00:00:00 2001 From: Pavan Kumar Reddy B Date: Tue, 23 Jun 2026 16:18:47 +0530 Subject: [PATCH 1/2] harden openssl verify, exec path and add regression tests --- source/jst_functions.c | 142 +++++++++++++++++++++++++-- tests/CMakeLists.txt | 12 +++ tests/openssl_verify_test.cpp | 180 ++++++++++++++++++++++++++++++++++ 3 files changed, 327 insertions(+), 7 deletions(-) create mode 100644 tests/openssl_verify_test.cpp diff --git a/source/jst_functions.c b/source/jst_functions.c index a09e7ad..c56549b 100644 --- a/source/jst_functions.c +++ b/source/jst_functions.c @@ -20,7 +20,9 @@ #include #include #include +#include #include +#include #include "jst_internal.h" #include "jst.h" @@ -42,6 +44,53 @@ typedef struct { size_t memsize; // allocated memory size (if applicable) } MemData; +static int has_shell_metachar(const char* command) +{ + static const char* kShellMetaChars = ";&|`$<>(){}[]*?!\\\n\r"; + return strpbrk(command, kShellMetaChars) != NULL; +} + +static int parse_exec_argv(char* command_buffer, char*** argv_out) +{ + size_t argc = 0; + size_t cap = 8; + char** argv = NULL; + char* saveptr = NULL; + char* token = NULL; + + argv = (char**)malloc(cap * sizeof(char*)); + if(!argv) + return 0; + + token = strtok_r(command_buffer, " \t", &saveptr); + while(token) + { + if(argc + 1 >= cap) + { + char** tmp = realloc(argv, (cap * 2) * sizeof(char*)); + if(!tmp) + { + free(argv); + return 0; + } + argv = tmp; + cap *= 2; + } + argv[argc++] = token; + token = strtok_r(NULL, " \t", &saveptr); + } + + if(argc == 0) + { + free(argv); + return 0; + } + + argv[argc] = NULL; + *argv_out = argv; + return 1; +} + static duk_ret_t do_getenv(duk_context *ctx) { @@ -148,25 +197,95 @@ static duk_ret_t do_gettext(duk_context *ctx) static duk_ret_t do_exec(duk_context *ctx) { char* command; + char* command_copy = NULL; + char** argv = NULL; + int pipefd[2] = {-1, -1}; + pid_t child_pid; + int status; + FILE* pipe = NULL; char *line = NULL; size_t len = 0; ssize_t nread; duk_idx_t idx; int index = 0; - idx = duk_push_array(ctx); - if (!parse_parameter(__FUNCTION__, ctx, "s", &command)) return 1; + if(!command || command[0] == '\0') + { + CosaPhpExtLog("exec rejected empty command\n"); + RETURN_FALSE; + } + + if(has_shell_metachar(command)) + { + CosaPhpExtLog("exec rejected unsafe command containing shell metacharacters\n"); + RETURN_FALSE; + } + CosaPhpExtLog("exec command=%s\n", command); - FILE* pipe = popen(command, "r"); + command_copy = strdup(command); + if(!command_copy) + { + CosaPhpExtLog("exec failed to allocate command copy\n"); + RETURN_FALSE; + } + + if(!parse_exec_argv(command_copy, &argv)) + { + CosaPhpExtLog("exec failed to parse command arguments\n"); + free(command_copy); + RETURN_FALSE; + } + + idx = duk_push_array(ctx); + + if(pipe(pipefd) != 0) + { + CosaPhpExtLog("exec failed to create pipe\n"); + free(argv); + free(command_copy); + duk_pop(ctx); + RETURN_FALSE; + } + + child_pid = fork(); + if(child_pid < 0) + { + CosaPhpExtLog("exec failed to fork\n"); + close(pipefd[0]); + close(pipefd[1]); + free(argv); + free(command_copy); + duk_pop(ctx); + RETURN_FALSE; + } + + if(child_pid == 0) + { + close(pipefd[0]); + if(dup2(pipefd[1], STDOUT_FILENO) < 0) + _exit(127); + close(pipefd[1]); + execvp(argv[0], argv); + _exit(127); + } + + close(pipefd[1]); + pipefd[1] = -1; + + pipe = fdopen(pipefd[0], "r"); if (!pipe) { - CosaPhpExtLog("exec failed to open pipe\n"); + CosaPhpExtLog("exec failed to open read stream\n"); + close(pipefd[0]); + free(argv); + free(command_copy); duk_pop(ctx); - return 1; + waitpid(child_pid, &status, 0); + RETURN_FALSE; } while((nread = getline(&line, &len, pipe)) != -1) @@ -177,7 +296,16 @@ static duk_ret_t do_exec(duk_context *ctx) } free(line); - pclose(pipe); + fclose(pipe); + + waitpid(child_pid, &status, 0); + if(WIFEXITED(status) && WEXITSTATUS(status) != 0) + { + CosaPhpExtLog("exec command exited with status=%d\n", WEXITSTATUS(status)); + } + + free(argv); + free(command_copy); return 1; } @@ -615,7 +743,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(strncmp(filepath, "file://", sizeof("file://")-1) != 0) { CosaPhpExtLog("openssl_verify_with_cert: file %s doesn't begin with 'file://'\n", filepath); free(sig_bytes); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 2628f71..c3a2c0b 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -91,6 +91,18 @@ endif(TEST_COMCAST_WEBUI) gtest_discover_tests(parser_test) +# testGroup.openssl_verify +add_executable( + openssl_verify_test + ../tests/main.cpp + ../tests/openssl_verify_test.cpp + ../source/jst_functions.c + ../source/jst_internal.c + ../source/duktape/duktape.c) +target_link_libraries(openssl_verify_test libgtest libgmock -pthread -lcrypto ${CURL_LIBRARIES}) + +gtest_discover_tests(openssl_verify_test) + #to run tests: # cd build/tests/parser # ../parser_test diff --git a/tests/openssl_verify_test.cpp b/tests/openssl_verify_test.cpp new file mode 100644 index 0000000..f3913f2 --- /dev/null +++ b/tests/openssl_verify_test.cpp @@ -0,0 +1,180 @@ +/* + If not stated otherwise in this file or this component's Licenses.txt file the + following copyright and licenses apply: + + Copyright 2018 RDK Management + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +#include "gtest/gtest.h" +#include +#include +#include + +extern "C" { +#include "jst.h" +duk_ret_t ccsp_functions_module_open(duk_context *ctx); +} + +class DuktapeCtxGuard { +public: + DuktapeCtxGuard() : ctx_(duk_create_heap_default()) { + } + + ~DuktapeCtxGuard() { + if (ctx_) { + duk_destroy_heap(ctx_); + } + } + + duk_context* get() const { + return ctx_; + } + +private: + duk_context* ctx_; +}; + +static bool call_openssl_verify_with_cert(duk_context* ctx, + const char* filepath, + const char* token, + const char* signature_base64url) +{ + duk_get_global_string(ctx, "ccsp"); + duk_get_prop_string(ctx, -1, "openssl_verify_with_cert"); + duk_push_string(ctx, filepath); + duk_push_string(ctx, token); + duk_push_string(ctx, signature_base64url); + duk_push_string(ctx, "RS256"); + + if (duk_pcall(ctx, 4) != DUK_EXEC_SUCCESS) { + duk_pop_2(ctx); + return false; + } + + bool ret = duk_get_boolean(ctx, -1); + duk_pop_2(ctx); + return ret; +} + +static bool call_exec_and_get_boolean_result(duk_context* ctx, + const char* command, + bool* result) +{ + duk_get_global_string(ctx, "ccsp"); + duk_get_prop_string(ctx, -1, "exec"); + duk_push_string(ctx, command); + + if (duk_pcall(ctx, 1) != DUK_EXEC_SUCCESS) { + duk_pop_2(ctx); + return false; + } + + if (!duk_is_boolean(ctx, -1)) { + duk_pop_2(ctx); + return false; + } + + *result = duk_get_boolean(ctx, -1) != 0; + duk_pop_2(ctx); + return true; +} + +static std::string create_temp_public_key_file() +{ + static const char kPublicKeyPem[] = + "-----BEGIN PUBLIC KEY-----\n" + "MFwwDQYJKoZIhvcNAQEBBQADSwAwSAJBALeJ8V+RaHcRUW2KIiMzFxLpy0X58F3R\n" + "o63eNbUsVTNff7kwh28ykVfoCENKz7LxyzKDn5XxhxL7sRKqzZo4PM8CAwEAAQ==\n" + "-----END PUBLIC KEY-----\n"; + + char tempName[L_tmpnam] = {0}; + if (std::tmpnam(tempName) == nullptr) { + return std::string(); + } + + std::ofstream out(tempName, std::ios::out | std::ios::trunc); + if (!out.is_open()) { + return std::string(); + } + + out << kPublicKeyPem; + out.close(); + + return std::string(tempName); +} + +TEST(openssl_verify_with_cert, handles_empty_filepath_without_crash) +{ + DuktapeCtxGuard guard; + duk_context* ctx = guard.get(); + + ASSERT_NE(ctx, nullptr); + + duk_push_c_function(ctx, ccsp_functions_module_open, 0); + duk_call(ctx, 0); + duk_put_global_string(ctx, "ccsp"); + + EXPECT_FALSE(call_openssl_verify_with_cert(ctx, "", "test-token", "AQ")); +} + +TEST(openssl_verify_with_cert, handles_short_filepath_without_crash) +{ + DuktapeCtxGuard guard; + duk_context* ctx = guard.get(); + + ASSERT_NE(ctx, nullptr); + + duk_push_c_function(ctx, ccsp_functions_module_open, 0); + duk_call(ctx, 0); + duk_put_global_string(ctx, "ccsp"); + + EXPECT_FALSE(call_openssl_verify_with_cert(ctx, "f", "test-token", "AQ")); +} + +TEST(openssl_verify_with_cert, rejects_invalid_signature_with_file_uri) +{ + DuktapeCtxGuard guard; + duk_context* ctx = guard.get(); + + ASSERT_NE(ctx, nullptr); + + duk_push_c_function(ctx, ccsp_functions_module_open, 0); + duk_call(ctx, 0); + duk_put_global_string(ctx, "ccsp"); + + std::string keyPath = create_temp_public_key_file(); + ASSERT_FALSE(keyPath.empty()); + + std::string fileUri = std::string("file://") + keyPath; + EXPECT_FALSE(call_openssl_verify_with_cert(ctx, fileUri.c_str(), "tokendata", "AQ")); + + std::remove(keyPath.c_str()); +} + +TEST(exec, rejects_shell_metacharacters) +{ + DuktapeCtxGuard guard; + duk_context* ctx = guard.get(); + + ASSERT_NE(ctx, nullptr); + + duk_push_c_function(ctx, ccsp_functions_module_open, 0); + duk_call(ctx, 0); + duk_put_global_string(ctx, "ccsp"); + + bool execResult = true; + ASSERT_TRUE(call_exec_and_get_boolean_result(ctx, "echo hello; id", &execResult)); + EXPECT_FALSE(execResult); +} \ No newline at end of file From 00d0c479b4c4ef4f06a70f787dea6bc9c89e63ad Mon Sep 17 00:00:00 2001 From: Pavan Kumar Reddy B Date: Tue, 23 Jun 2026 16:55:06 +0530 Subject: [PATCH 2/2] harden child process handling and resolve pipe stream naming issues --- source/jst_functions.c | 37 +++++++++++++++++++++++++---------- tests/openssl_verify_test.cpp | 11 +++++++---- 2 files changed, 34 insertions(+), 14 deletions(-) diff --git a/source/jst_functions.c b/source/jst_functions.c index c56549b..bdd67e9 100644 --- a/source/jst_functions.c +++ b/source/jst_functions.c @@ -91,6 +91,19 @@ static int parse_exec_argv(char* command_buffer, char*** argv_out) return 1; } +static int wait_for_child_process(pid_t child_pid, int* status_out) +{ + int rc; + + do + { + rc = waitpid(child_pid, status_out, 0); + } + while(rc == -1 && errno == EINTR); + + return rc; +} + static duk_ret_t do_getenv(duk_context *ctx) { @@ -202,13 +215,15 @@ static duk_ret_t do_exec(duk_context *ctx) int pipefd[2] = {-1, -1}; pid_t child_pid; int status; - FILE* pipe = NULL; + FILE* pipe_stream = NULL; char *line = NULL; size_t len = 0; ssize_t nread; duk_idx_t idx; int index = 0; + idx = duk_push_array(ctx); + if (!parse_parameter(__FUNCTION__, ctx, "s", &command)) return 1; @@ -240,8 +255,6 @@ static duk_ret_t do_exec(duk_context *ctx) RETURN_FALSE; } - idx = duk_push_array(ctx); - if(pipe(pipefd) != 0) { CosaPhpExtLog("exec failed to create pipe\n"); @@ -276,19 +289,20 @@ static duk_ret_t do_exec(duk_context *ctx) close(pipefd[1]); pipefd[1] = -1; - pipe = fdopen(pipefd[0], "r"); - if (!pipe) + pipe_stream = fdopen(pipefd[0], "r"); + if (!pipe_stream) { CosaPhpExtLog("exec failed to open read stream\n"); close(pipefd[0]); free(argv); free(command_copy); duk_pop(ctx); - waitpid(child_pid, &status, 0); + if(wait_for_child_process(child_pid, &status) == -1) + CosaPhpExtLog("exec waitpid failed: %s\n", strerror(errno)); RETURN_FALSE; } - while((nread = getline(&line, &len, pipe)) != -1) + while((nread = getline(&line, &len, pipe_stream)) != -1) { CosaPhpExtLog("exec line: %s\n", line); duk_push_string(ctx, line); @@ -296,10 +310,13 @@ static duk_ret_t do_exec(duk_context *ctx) } free(line); - fclose(pipe); + fclose(pipe_stream); - waitpid(child_pid, &status, 0); - if(WIFEXITED(status) && WEXITSTATUS(status) != 0) + if(wait_for_child_process(child_pid, &status) == -1) + { + CosaPhpExtLog("exec waitpid failed: %s\n", strerror(errno)); + } + else if(WIFEXITED(status) && WEXITSTATUS(status) != 0) { CosaPhpExtLog("exec command exited with status=%d\n", WEXITSTATUS(status)); } diff --git a/tests/openssl_verify_test.cpp b/tests/openssl_verify_test.cpp index f3913f2..c55c303 100644 --- a/tests/openssl_verify_test.cpp +++ b/tests/openssl_verify_test.cpp @@ -21,6 +21,7 @@ #include #include #include +#include extern "C" { #include "jst.h" @@ -99,12 +100,14 @@ static std::string create_temp_public_key_file() "o63eNbUsVTNff7kwh28ykVfoCENKz7LxyzKDn5XxhxL7sRKqzZo4PM8CAwEAAQ==\n" "-----END PUBLIC KEY-----\n"; - char tempName[L_tmpnam] = {0}; - if (std::tmpnam(tempName) == nullptr) { + char tempTemplate[] = "/tmp/jst_pubkey_XXXXXX"; + int fd = mkstemp(tempTemplate); + if (fd < 0) { return std::string(); } + close(fd); - std::ofstream out(tempName, std::ios::out | std::ios::trunc); + std::ofstream out(tempTemplate, std::ios::out | std::ios::trunc); if (!out.is_open()) { return std::string(); } @@ -112,7 +115,7 @@ static std::string create_temp_public_key_file() out << kPublicKeyPem; out.close(); - return std::string(tempName); + return std::string(tempTemplate); } TEST(openssl_verify_with_cert, handles_empty_filepath_without_crash)