diff --git a/source/jst_internal.c b/source/jst_internal.c index e7312f0..3de8de2 100644 --- a/source/jst_internal.c +++ b/source/jst_internal.c @@ -164,11 +164,27 @@ int read_file(const char *filename, char** bufout, size_t* lenout) { FILE* pf; size_t size; + long tell_result; size_t rc; char* buf; + if(!bufout || !lenout) + { + CosaPhpExtLog("read_file invalid output buffer for %s\n", filename ? filename : "(null)"); + return 0; + } + + *bufout = NULL; + *lenout = 0; + CosaPhpExtLog( "read_file %s\n", filename ); + if(!filename) + { + fprintf(stderr, "Error: invalid filename\n"); + return 0; + } + errno = 0; pf = fopen(filename, "r"); if(!pf) @@ -179,9 +195,35 @@ int read_file(const char *filename, char** bufout, size_t* lenout) return 0; } - fseek(pf, 0, SEEK_END); - size = ftell(pf); - rewind(pf); + if(fseek(pf, 0, SEEK_END) != 0) + { + fclose(pf); + fprintf(stderr, "Error: seek failed %s\n", filename); + return 0; + } + + tell_result = ftell(pf); + if(tell_result < 0) + { + fclose(pf); + fprintf(stderr, "Error: tell failed %s\n", filename); + return 0; + } + + size = (size_t)tell_result; + if ((long)size != tell_result || size == (size_t)-1) + { + fclose(pf); + fprintf(stderr, "Error: file too large %s\n", filename); + return 0; + } + + if(fseek(pf, 0, SEEK_SET) != 0) + { + fclose(pf); + fprintf(stderr, "Error: rewind failed %s\n", filename); + return 0; + } buf = (char*)calloc(size+1, 1); if(!buf) diff --git a/source/jst_session.c b/source/jst_session.c index 69f542f..8a827cd 100644 --- a/source/jst_session.c +++ b/source/jst_session.c @@ -102,6 +102,9 @@ static duk_ret_t session_start(duk_context *ctx) CosaPhpExtLog("%s: cookie %s\n", __PRETTY_FUNCTION__, cookie); /*load session id from cookie*/ const char* sesid = NULL; + const char* sesid_end = NULL; + size_t sesid_len = 0; + char sesid_copy[SESSION_ID_LENGTH + 1]; const char* tmp = cookie; while (tmp = strstr(tmp, "DUKSID=")) { @@ -112,14 +115,29 @@ static duk_ret_t session_start(duk_context *ctx) if(sesid) { sesid += 7; - int len = strlen(sesid); - if(len >= SESSION_ID_LENGTH) + sesid_end = strchr(sesid, ';'); + if(sesid_end) + sesid_len = (size_t)(sesid_end - sesid); + else + sesid_len = strlen(sesid); + + if(sesid_len == SESSION_ID_LENGTH) { + memcpy(sesid_copy, sesid, SESSION_ID_LENGTH); + sesid_copy[SESSION_ID_LENGTH] = '\0'; + int idx = SESSION_PREFIX_LEN; int isvalid = 1; + + if(strncmp(sesid_copy, SESSION_PREFIX, SESSION_PREFIX_LEN) != 0) + { + CosaPhpExtLog("Invalid SessionID prefix\n"); + isvalid = 0; + } + /* Validate session ID*/ while ( idx < SESSION_ID_LENGTH) { - if (!isalnum(sesid[idx])) { + if (!isalnum((unsigned char)sesid_copy[idx])) { CosaPhpExtLog("Invalid SessionID\n"); isvalid = 0; break; @@ -128,26 +146,27 @@ static duk_ret_t session_start(duk_context *ctx) } if(isvalid) { - sesid = strtok(sesid, ";"); - const char filename[SESSION_FILE_MAX_PATH]; - snprintf(filename, SESSION_FILE_MAX_PATH, "%s/%s", SESSION_TMP_DIR, sesid); + char filename[SESSION_FILE_MAX_PATH]; + snprintf(filename, SESSION_FILE_MAX_PATH, "%s/%s", SESSION_TMP_DIR, sesid_copy); 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_copy, SESSION_ID_LENGTH); } else { CosaPhpExtLog("%s: Failed to read Session file %s\n", __PRETTY_FUNCTION__, filename); } } } else { - CosaPhpExtLog("Invalid SessionID Entropy\n"); + CosaPhpExtLog("Invalid SessionID Entropy\n"); } } } if(!session_identifier[0]) { CosaPhpExtLog("Invalid Session\n"); + free(session_identifier); + session_identifier = NULL; RETURN_FALSE; } @@ -304,7 +323,7 @@ static duk_ret_t session_get_data(duk_context *ctx) /*check if we are at end of file content, ignoring whitespace*/ for(j = i+1; j < content_len; ++j) { - if(!isspace(s1[j])) + if(!isspace((unsigned char)s1[j])) { /*more content found*/ break; diff --git a/tests/parser_test.cpp b/tests/parser_test.cpp index a3bc427..b523dad 100644 --- a/tests/parser_test.cpp +++ b/tests/parser_test.cpp @@ -20,6 +20,8 @@ #include #include #include +#include +#include #include #include "jst.h" #include @@ -28,6 +30,7 @@ #include extern "C" { + int read_file(const char *filename, char** bufout, size_t* lenout); duk_ret_t ccsp_session_module_open(duk_context *ctx); } @@ -46,6 +49,37 @@ class BufferFreer char* buffer_; }; +class EnvVarGuard +{ +public: + explicit EnvVarGuard(const char* name) + : name_(name), had_value_(false) + { + const char* value = getenv(name_); + if (value) + { + old_value_ = value; + had_value_ = true; + } + } + + ~EnvVarGuard() + { + if (had_value_) + setenv(name_, old_value_.c_str(), 1); + else + unsetenv(name_); + } + + EnvVarGuard(const EnvVarGuard&) = delete; + EnvVarGuard& operator=(const EnvVarGuard&) = delete; + +private: + const char* name_; + std::string old_value_; + bool had_value_; +}; + int recurseDirectory(const string& path, vector& files, const string& match) { DIR *dir; @@ -111,6 +145,16 @@ TEST(general, parser) { } } +TEST(general, read_file_directory_input_returns_failure) +{ + char* buffer = reinterpret_cast(0x1); + size_t length = 123; + + EXPECT_EQ(read_file("//", &buffer, &length), 0); + EXPECT_EQ(buffer, nullptr); + EXPECT_EQ(length, 0u); +} + TEST(general, session_create_multiple_calls_succeed) { duk_context* ctx = duk_create_heap_default(); @@ -190,6 +234,93 @@ TEST(general, session_create_destroy_cycle_and_id_format) duk_destroy_heap(ctx); } +TEST(general, session_start_does_not_modify_cookie_env) +{ + EnvVarGuard http_cookie_guard("HTTP_COOKIE"); + + const char* sesid = "jst_sessABCDEFGHIJKLMNOPQRSTUVWXYZ012345"; + char session_file_path[128]; + char cookie[256]; + + snprintf(session_file_path, sizeof(session_file_path), "/tmp/%s", sesid); + FILE* f = fopen(session_file_path, "w"); + ASSERT_NE(f, nullptr); + fclose(f); + + snprintf(cookie, sizeof(cookie), "a=1; DUKSID=%s; b=2", sesid); + ASSERT_EQ(setenv("HTTP_COOKIE", cookie, 1), 0); + + duk_context* ctx = duk_create_heap_default(); + ASSERT_NE(ctx, nullptr); + + duk_push_c_function(ctx, ccsp_session_module_open, 0); + ASSERT_EQ(duk_pcall(ctx, 0), DUK_EXEC_SUCCESS); + duk_put_global_string(ctx, "ccsp_session"); + + duk_get_global_string(ctx, "ccsp_session"); + duk_get_prop_string(ctx, -1, "destroy"); + ASSERT_EQ(duk_pcall(ctx, 0), DUK_EXEC_SUCCESS); + duk_pop_2(ctx); + + duk_get_global_string(ctx, "ccsp_session"); + duk_get_prop_string(ctx, -1, "start"); + ASSERT_EQ(duk_pcall(ctx, 0), DUK_EXEC_SUCCESS); + ASSERT_TRUE(duk_get_boolean(ctx, -1)); + duk_pop_2(ctx); + + const char* cookie_after = getenv("HTTP_COOKIE"); + ASSERT_NE(cookie_after, nullptr); + EXPECT_STREQ(cookie_after, cookie); + + duk_get_global_string(ctx, "ccsp_session"); + duk_get_prop_string(ctx, -1, "destroy"); + ASSERT_EQ(duk_pcall(ctx, 0), DUK_EXEC_SUCCESS); + duk_pop_2(ctx); + + duk_destroy_heap(ctx); + + remove(session_file_path); +} + +TEST(general, session_start_rejects_invalid_session_prefix) +{ + EnvVarGuard http_cookie_guard("HTTP_COOKIE"); + + const char* invalid_sesid = "bad_prefABCDEFGHIJKLMNOPQRSTUVWXYZ012345"; + char session_file_path[128]; + char cookie[256]; + + snprintf(session_file_path, sizeof(session_file_path), "/tmp/%s", invalid_sesid); + FILE* f = fopen(session_file_path, "w"); + ASSERT_NE(f, nullptr); + fclose(f); + + snprintf(cookie, sizeof(cookie), "DUKSID=%s", invalid_sesid); + ASSERT_EQ(setenv("HTTP_COOKIE", cookie, 1), 0); + + duk_context* ctx = duk_create_heap_default(); + ASSERT_NE(ctx, nullptr); + + duk_push_c_function(ctx, ccsp_session_module_open, 0); + ASSERT_EQ(duk_pcall(ctx, 0), DUK_EXEC_SUCCESS); + duk_put_global_string(ctx, "ccsp_session"); + + duk_get_global_string(ctx, "ccsp_session"); + duk_get_prop_string(ctx, -1, "destroy"); + ASSERT_EQ(duk_pcall(ctx, 0), DUK_EXEC_SUCCESS); + duk_pop_2(ctx); + + duk_get_global_string(ctx, "ccsp_session"); + duk_get_prop_string(ctx, -1, "start"); + ASSERT_EQ(duk_pcall(ctx, 0), DUK_EXEC_SUCCESS); + EXPECT_FALSE(duk_get_boolean(ctx, -1)); + duk_pop_2(ctx); + + duk_destroy_heap(ctx); + + remove(session_file_path); +} + int main(int argc, char* argv[]) { ::testing::InitGoogleTest(&argc, argv);