From b48c21d244092658d6e2d1bb243b705fd968b9f7 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Tue, 2 Jun 2020 13:11:14 +0200 Subject: [PATCH] fix: Avoid memory unsafety when loading session from disk (#270) --- src/sentry_session.c | 27 +++++++++++++++++++++-- src/sentry_session.h | 4 ++-- tests/test_integration_stdout.py | 38 ++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 4 deletions(-) diff --git a/src/sentry_session.c b/src/sentry_session.c index b21f6a26f..f75e30eec 100644 --- a/src/sentry_session.c +++ b/src/sentry_session.c @@ -51,13 +51,19 @@ sentry__session_new(void) if (!opts) { return NULL; } - const char *release = sentry_options_get_release(opts); - const char *environment = sentry_options_get_environment(opts); + char *release = sentry__string_clone(sentry_options_get_release(opts)); if (!release) { return NULL; } sentry_session_t *rv = SENTRY_MAKE(sentry_session_t); + if (!rv) { + return NULL; + } + + char *environment + = sentry__string_clone(sentry_options_get_environment(opts)); + rv->release = release; rv->environment = environment; rv->session_id = sentry_uuid_new_v4(); @@ -78,6 +84,8 @@ sentry__session_free(sentry_session_t *session) return; } sentry_value_decref(session->distinct_id); + sentry_free(session->release); + sentry_free(session->environment); sentry_free(session); } @@ -139,6 +147,16 @@ sentry__session_from_json(const char *buf, size_t buflen) return NULL; } + sentry_value_t attrs = sentry_value_get_by_key(value, "attrs"); + if (sentry_value_is_null(attrs)) { + return NULL; + } + char *release = sentry__string_clone( + sentry_value_as_string(sentry_value_get_by_key(attrs, "release"))); + if (!release) { + return NULL; + } + sentry_session_t *rv = SENTRY_MAKE(sentry_session_t); if (!rv) { return NULL; @@ -148,11 +166,16 @@ sentry__session_from_json(const char *buf, size_t buflen) rv->distinct_id = sentry_value_get_by_key_owned(value, "did"); + rv->release = release; + rv->environment = sentry__string_clone( + sentry_value_as_string(sentry_value_get_by_key(attrs, "environment"))); + const char *status = sentry_value_as_string(sentry_value_get_by_key(value, "status")); rv->status = status_from_string(status); rv->init = sentry_value_is_true(sentry_value_get_by_key(value, "init")); + rv->errors = (int64_t)sentry_value_as_int32( sentry_value_get_by_key(value, "errors")); rv->started_ms = sentry__iso8601_to_msec( diff --git a/src/sentry_session.h b/src/sentry_session.h index 7e52722e7..7c1c1a9f7 100644 --- a/src/sentry_session.h +++ b/src/sentry_session.h @@ -20,8 +20,8 @@ typedef enum { * metadata. */ typedef struct sentry_session_s { - const char *release; - const char *environment; + char *release; + char *environment; sentry_uuid_t session_id; sentry_value_t distinct_id; uint64_t started_ms; diff --git a/tests/test_integration_stdout.py b/tests/test_integration_stdout.py index 6687237ef..c12f7f078 100644 --- a/tests/test_integration_stdout.py +++ b/tests/test_integration_stdout.py @@ -3,6 +3,7 @@ import sys import os import time +import json from . import cmake, check_output, run, Envelope from .conditions import has_inproc, has_breakpad, has_files from .assertions import ( @@ -14,6 +15,7 @@ assert_crash, assert_minidump, assert_timestamp, + assert_session, ) @@ -115,6 +117,42 @@ def test_multi_process(tmp_path): assert len(runs) == 0 +@pytest.mark.skipif(not has_files, reason="test needs a local filesystem") +def test_abnormal_session(tmp_path): + cmake( + tmp_path, + ["sentry_example"], + {"SENTRY_BACKEND": "none", "SENTRY_TRANSPORT": "none"}, + ) + + # create a bogus session file + db_dir = tmp_path.joinpath(".sentry-native") + db_dir.mkdir() + run_dir = db_dir.joinpath("foobar.run") + run_dir.mkdir() + with open(run_dir.joinpath("session.json"), "w") as session_file: + json.dump( + { + "sid": "00000000-0000-0000-0000-000000000000", + "did": "42", + "status": "started", + "errors": 0, + "started": "2020-06-02T10:04:53.680Z", + "duration": 10, + "attrs": { + "release": "test-example-release", + "environment": "development", + }, + }, + session_file, + ) + + output = check_output(tmp_path, "sentry_example", ["stdout", "no-setup"]) + envelope = Envelope.deserialize(output) + + assert_session(envelope, {"status": "abnormal", "errors": 0, "duration": 10}) + + @pytest.mark.skipif(not has_inproc, reason="test needs inproc backend") def test_inproc_crash_stdout(tmp_path): cmake(