Skip to content

Commit

Permalink
fix: Avoid memory unsafety when loading session from disk (#270)
Browse files Browse the repository at this point in the history
  • Loading branch information
Swatinem authored Jun 2, 2020
1 parent 5dd159f commit b48c21d
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 4 deletions.
27 changes: 25 additions & 2 deletions src/sentry_session.c
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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);
}

Expand Down Expand Up @@ -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;
Expand All @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions src/sentry_session.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
38 changes: 38 additions & 0 deletions tests/test_integration_stdout.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -14,6 +15,7 @@
assert_crash,
assert_minidump,
assert_timestamp,
assert_session,
)


Expand Down Expand Up @@ -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(
Expand Down

0 comments on commit b48c21d

Please sign in to comment.