diff --git a/source/jst_internal.c b/source/jst_internal.c index e7312f0..bcc4532 100644 --- a/source/jst_internal.c +++ b/source/jst_internal.c @@ -164,30 +164,74 @@ int read_file(const char *filename, char** bufout, size_t* lenout) { FILE* pf; size_t size; + long tell_result; size_t rc; char* buf; + const char* safe_filename = (filename && filename[0]) ? filename : ""; - CosaPhpExtLog( "read_file %s\n", filename ); + if(!bufout || !lenout) + { + CosaPhpExtLog("read_file invalid output buffer for %s\n", safe_filename); + return 0; + } + + *bufout = NULL; + *lenout = 0; + + CosaPhpExtLog("read_file %s\n", safe_filename); + + if(!filename || !filename[0]) + { + CosaPhpExtLog("read_file invalid filename %s\n", safe_filename); + fprintf(stderr, "Error: invalid filename %s\n", safe_filename); + return 0; + } errno = 0; pf = fopen(filename, "r"); if(!pf) { char * serr = strerror(errno); - CosaPhpExtLog( "read_file cannot open file:%s error:%s\n", filename, serr ); - fprintf(stderr, "Error: cannot open file:%s error:%s\n", filename, serr); + CosaPhpExtLog( "read_file cannot open file:%s error:%s\n", safe_filename, serr ); + fprintf(stderr, "Error: cannot open file:%s error:%s\n", safe_filename, serr); + return 0; + } + + if(fseek(pf, 0, SEEK_END) != 0) + { + fclose(pf); + fprintf(stderr, "Error: seek failed %s\n", safe_filename); return 0; } - fseek(pf, 0, SEEK_END); - size = ftell(pf); - rewind(pf); + tell_result = ftell(pf); + if(tell_result < 0) + { + fclose(pf); + fprintf(stderr, "Error: tell failed %s\n", safe_filename); + return 0; + } + + size = (size_t)tell_result; + if ((long)size != tell_result || size == (size_t)-1 || (size_t)(int)size != size) + { + fclose(pf); + fprintf(stderr, "Error: file too large %s\n", safe_filename); + return 0; + } + + if(fseek(pf, 0, SEEK_SET) != 0) + { + fclose(pf); + fprintf(stderr, "Error: rewind failed %s\n", safe_filename); + return 0; + } buf = (char*)calloc(size+1, 1); if(!buf) { fclose(pf); - fprintf(stderr, "Error: malloc oom %s\n", filename); + fprintf(stderr, "Error: malloc oom %s\n", safe_filename); return 0; } @@ -196,7 +240,7 @@ int read_file(const char *filename, char** bufout, size_t* lenout) { free(buf); fclose(pf); - fprintf(stderr, "Error: read failed %s\n", filename); + fprintf(stderr, "Error: read failed %s\n", safe_filename); return 0; } diff --git a/source/jst_session.c b/source/jst_session.c index 69f542f..3a15b82 100644 --- a/source/jst_session.c +++ b/source/jst_session.c @@ -102,24 +102,49 @@ 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=")) + while ((tmp = strstr(tmp, "DUKSID="))) { sesid= tmp; tmp++; } - CosaPhpExtLog("%s: sesid %s\n", __PRETTY_FUNCTION__, sesid); if(sesid) { + CosaPhpExtLog("%s: sesid %s\n", __PRETTY_FUNCTION__, 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); + } + else + { + CosaPhpExtLog("%s: sesid \n", __PRETTY_FUNCTION__); + } + + if(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 +153,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 +330,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 897e536..f92a808 100644 --- a/tests/parser_test.cpp +++ b/tests/parser_test.cpp @@ -31,6 +31,7 @@ #include extern "C" { + int read_file(const char *filename, char** bufout, size_t* lenout); duk_ret_t ccsp_post_module_open(duk_context *ctx); duk_ret_t ccsp_session_module_open(duk_context *ctx); } @@ -72,6 +73,9 @@ class EnvVarGuard unsetenv(name_); } + EnvVarGuard(const EnvVarGuard&) = delete; + EnvVarGuard& operator=(const EnvVarGuard&) = delete; + void set(const char* value) { if (value) @@ -214,6 +218,21 @@ TEST(general, parser) { } } +TEST(general, read_file_directory_input_returns_failure) +{ + char temp_dir_template[] = "/tmp/jst_read_file_directory_input_XXXXXX"; + char* buffer = reinterpret_cast(0x1); + size_t length = 123; + char* temp_dir = mkdtemp(temp_dir_template); + + ASSERT_NE(temp_dir, nullptr); + + EXPECT_EQ(read_file(temp_dir, &buffer, &length), 0); + EXPECT_EQ(buffer, nullptr); + EXPECT_EQ(length, 0u); + EXPECT_EQ(rmdir(temp_dir), 0); +} + TEST(general, session_create_multiple_calls_succeed) { duk_context* ctx = duk_create_heap_default(); @@ -341,6 +360,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);