Skip to content

Commit

Permalink
Amend pr comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@estringana
estringana committed Aug 6, 2024
1 parent c224f9f commit 5668367
Show file tree
Hide file tree
Showing 11 changed files with 78 additions and 92 deletions.
91 changes: 55 additions & 36 deletions appsec/src/extension/backtrace.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
#include "string_helpers.h"

static const int NO_LIMIT = 0;
static const double STACK_DEFAULT_TOP_PERCENTAGE = 0.25;
static const char *QUALIFIED_NAME_SEPARATOR = "::";
static const double STACK_DEFAULT_TOP_RATE = 0.25;
static const char QUALIFIED_NAME_SEPARATOR[] = "::";

static zend_string *_frames_key;
static zend_string *_language_key;
Expand All @@ -24,20 +24,24 @@ static zend_string *_frame_line;
static zend_string *_frame_function;
static zend_string *_frame_file;
static zend_string *_id_key;
static zend_string *_line_field;
static zend_string *_function_field;
static zend_string *_file_field;
static zend_string *_class_field;

bool php_backtrace_frame_to_datadog_backtrace_frame( // NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
static bool
php_backtrace_frame_to_datadog_backtrace_frame( // NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
zval *nonnull php_backtrace_frame, zval *nonnull datadog_backtrace_frame,
zend_ulong index)
zend_long index)
{
if (Z_TYPE_P(php_backtrace_frame) != IS_ARRAY) {
return false;
}
HashTable *frame = Z_ARRVAL_P(php_backtrace_frame);
zval *line = zend_hash_str_find(frame, "line", sizeof("line") - 1);
zval *function =
zend_hash_str_find(frame, "function", sizeof("function") - 1);
zval *file = zend_hash_str_find(frame, "file", sizeof("file") - 1);
zval *class = zend_hash_str_find(frame, "class", sizeof("class") - 1);
zval *line = zend_hash_find(frame, _line_field);
zval *function = zend_hash_find(frame, _function_field);
zval *file = zend_hash_find(frame, _file_field);
zval *class = zend_hash_find(frame, _class_field);
zval id;
ZVAL_LONG(&id, index);
#ifdef TESTING
Expand Down Expand Up @@ -66,17 +70,18 @@ bool php_backtrace_frame_to_datadog_backtrace_frame( // NOLINTNEXTLINE(bugprone-
}

int qualified_name_size = Z_STRLEN_P(function);
int qualified_name_separator_len = strlen(QUALIFIED_NAME_SEPARATOR);
qualified_name_size +=
class ? Z_STRLEN_P(class) + qualified_name_separator_len : 0;
char *qualified_name = safe_emalloc(qualified_name_size, 1, 1);
class ? Z_STRLEN_P(class) + sizeof(QUALIFIED_NAME_SEPARATOR) - 1 : 0;
zend_string *qualified_name_zstr =
zend_string_alloc(qualified_name_size, 0);
char *qualified_name = ZSTR_VAL(qualified_name_zstr);
int position = 0;

if (class) {
memcpy(qualified_name, Z_STRVAL_P(class), Z_STRLEN_P(class));
position = Z_STRLEN_P(class);
memcpy(&qualified_name[position], QUALIFIED_NAME_SEPARATOR,
qualified_name_separator_len);
sizeof(QUALIFIED_NAME_SEPARATOR) - 1);
position += 2;
}

Expand All @@ -86,10 +91,9 @@ bool php_backtrace_frame_to_datadog_backtrace_frame( // NOLINTNEXTLINE(bugprone-
qualified_name[qualified_name_size] = '\0';

zval zv_qualified_name;
ZVAL_STRING(&zv_qualified_name, qualified_name);
ZVAL_STR(&zv_qualified_name, qualified_name_zstr);
zend_hash_add(
datadog_backtrace_frame_ht, _frame_function, &zv_qualified_name);
efree(qualified_name);

if (file) {
zend_hash_add(datadog_backtrace_frame_ht, _frame_file, file);
Expand All @@ -100,7 +104,7 @@ bool php_backtrace_frame_to_datadog_backtrace_frame( // NOLINTNEXTLINE(bugprone-
return true;
}

void php_backtrace_to_datadog_backtrace(
static void php_backtrace_to_datadog_backtrace(
// NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
zval *nonnull php_backtrace, zval *nonnull datadog_backtrace)
{
Expand All @@ -109,34 +113,34 @@ void php_backtrace_to_datadog_backtrace(
}

HashTable *php_backtrace_ht = Z_ARRVAL_P(php_backtrace);
unsigned int frames_on_stack = zend_array_count(php_backtrace_ht);
uint32_t frames_on_stack = zend_array_count(php_backtrace_ht);

unsigned int top = frames_on_stack;
unsigned int bottom = 0;
uint32_t top = frames_on_stack;
uint32_t bottom = 0;
if (get_global_DD_APPSEC_MAX_STACK_TRACE_DEPTH() != 0 &&
get_global_DD_APPSEC_MAX_STACK_TRACE_DEPTH() < frames_on_stack) {
top = (unsigned int)round(
frames_on_stack > get_global_DD_APPSEC_MAX_STACK_TRACE_DEPTH()) {
top = (uint32_t)round(
(double)get_global_DD_APPSEC_MAX_STACK_TRACE_DEPTH() *
STACK_DEFAULT_TOP_PERCENTAGE);
STACK_DEFAULT_TOP_RATE);
bottom = get_global_DD_APPSEC_MAX_STACK_TRACE_DEPTH() - top;
}

array_init(datadog_backtrace);

HashTable *datadog_backtrace_ht = Z_ARRVAL_P(datadog_backtrace);

zval *tmp;
zval *php_frame;
zend_ulong index;
if (top > 0) {
ZEND_HASH_FOREACH_NUM_KEY_VAL(php_backtrace_ht, index, tmp)
ZEND_HASH_FOREACH_NUM_KEY_VAL(php_backtrace_ht, index, php_frame)
{
zval new_frame;

if (!php_backtrace_frame_to_datadog_backtrace_frame(
tmp, &new_frame, index)) {
continue;
if (php_backtrace_frame_to_datadog_backtrace_frame(
php_frame, &new_frame, index)) {
zend_hash_next_index_insert_new(
datadog_backtrace_ht, &new_frame);
}
zend_hash_next_index_insert_new(datadog_backtrace_ht, &new_frame);
if (--top == 0) {
break;
}
Expand All @@ -148,11 +152,11 @@ void php_backtrace_to_datadog_backtrace(
unsigned int position = frames_on_stack - bottom;
DD_FOREACH_FROM(php_backtrace_ht, 0, position, index)
{
tmp = _z;
php_frame = _z;
zval new_frame;

if (!php_backtrace_frame_to_datadog_backtrace_frame(
tmp, &new_frame, index)) {
php_frame, &new_frame, index)) {
continue;
}

Expand Down Expand Up @@ -224,25 +228,36 @@ bool dd_report_exploit_backtrace(zend_string *nullable id)
return false;
}

if (Z_TYPE_P(meta_struct) != IS_ARRAY) {
if (Z_TYPE_P(meta_struct) == IS_NULL) {
array_init(meta_struct);
} else if (Z_TYPE_P(meta_struct) != IS_ARRAY) {
return false;
}

zval *dd_stack = dd_hash_find_or_new(Z_ARR_P(meta_struct), _dd_stack_key);
if (Z_TYPE_P(dd_stack) != IS_ARRAY) {
zval *dd_stack = zend_hash_find(Z_ARR_P(meta_struct), _dd_stack_key);
zval *exploit = NULL;
if (!dd_stack || Z_TYPE_P(dd_stack) == IS_NULL) {
dd_stack = zend_hash_add_new(
Z_ARR_P(meta_struct), _dd_stack_key, &EG(uninitialized_zval));
array_init(dd_stack);
exploit = zend_hash_add_new(
Z_ARR_P(dd_stack), _exploit_key, &EG(uninitialized_zval));
array_init(exploit);
} else if (Z_TYPE_P(dd_stack) != IS_ARRAY) {
return false;
} else {
exploit = zend_hash_find(Z_ARR_P(dd_stack), _exploit_key);
}

zval *exploit = dd_hash_find_or_new(Z_ARR_P(dd_stack), _exploit_key);
if (Z_TYPE_P(exploit) != IS_ARRAY) {
array_init(exploit);
return false;
}

unsigned int limit = get_global_DD_APPSEC_MAX_STACK_TRACES();
if (limit != 0 && zend_array_count(Z_ARR_P(exploit)) == limit) {
mlog(dd_log_debug,
"Stacktrace not generated due to limit "
"D_APPSEC_MAX_STACK_TRACES(%d) has been reached",
"DD_APPSEC_MAX_STACK_TRACES(%u) has been reached",
limit);
return false;
}
Expand Down Expand Up @@ -310,6 +325,10 @@ void dd_backtrace_startup()
_frame_function = zend_string_init_interned(LSTRARG("function"), 1);
_frame_file = zend_string_init_interned(LSTRARG("file"), 1);
_id_key = zend_string_init_interned(LSTRARG("id"), 1);
_line_field = zend_string_init_interned(LSTRARG("line"), 1);
_file_field = zend_string_init_interned(LSTRARG("file"), 1);
_function_field = zend_string_init_interned(LSTRARG("function"), 1);
_class_field = zend_string_init_interned(LSTRARG("class"), 1);
#ifdef TESTING
_register_testing_objects();
#endif
Expand Down
15 changes: 1 addition & 14 deletions appsec/src/extension/php_helpers.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,17 +114,4 @@ zend_string *nullable dd_php_get_string_elem(
}

return Z_STR_P(zresult);
}

zval *nonnull dd_hash_find_or_new(
HashTable *nonnull ht, zend_string *nonnull key)
{
zval *result = zend_hash_find(ht, key);

if (!result) {
zval new_zv;
result = zend_hash_add(ht, key, &new_zv);
}

return result;
}
}
2 changes: 0 additions & 2 deletions appsec/src/extension/php_helpers.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,5 +47,3 @@ zend_string *nullable dd_php_get_string_elem(
const zend_array *nullable arr, zend_string *nonnull zstr);
zend_string *nullable dd_php_get_string_elem_cstr(
const zend_array *nullable arr, const char *nonnull name, size_t len);
zval *nonnull dd_hash_find_or_new(
HashTable *nonnull ht, zend_string *nonnull key);
8 changes: 3 additions & 5 deletions appsec/tests/extension/generate_backtrace_06.phpt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ namespace DDTrace {
namespace {
include __DIR__ . '/inc/ddtrace_version.php';

ddtrace_version_at_least('0.79.0');

function two($param01, $param02)
{
var_dump(ltrim(" Verify the wrapped function works"));
Expand Down Expand Up @@ -58,7 +56,7 @@ array(3) {
[0]=>
array(4) {
["line"]=>
int(26)
int(24)
["function"]=>
string(5) "ltrim"
["file"]=>
Expand All @@ -69,7 +67,7 @@ array(3) {
[1]=>
array(4) {
["line"]=>
int(31)
int(29)
["function"]=>
string(3) "two"
["file"]=>
Expand All @@ -80,7 +78,7 @@ array(3) {
[2]=>
array(4) {
["line"]=>
int(39)
int(37)
["function"]=>
string(3) "one"
["file"]=>
Expand Down
4 changes: 1 addition & 3 deletions appsec/tests/extension/generate_backtrace_07.phpt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ namespace Some\NameSpace {
namespace {
include __DIR__ . '/inc/ddtrace_version.php';

ddtrace_version_at_least('0.79.0');

DDTrace\start_span();
$root = DDTrace\active_span();

Expand Down Expand Up @@ -71,7 +69,7 @@ array(3) {
[2]=>
array(4) {
["line"]=>
int(31)
int(29)
["function"]=>
string(23) "Some\NameSpace\Foo::one"
["file"]=>
Expand Down
5 changes: 1 addition & 4 deletions appsec/tests/extension/report_backtrace_01.phpt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,6 @@ extension=ddtrace.so
<?php
include __DIR__ . '/inc/ddtrace_version.php';

ddtrace_version_at_least('0.79.0');


use function datadog\appsec\testing\report_exploit_backtrace;

function two($param01, $param02)
Expand Down Expand Up @@ -40,5 +37,5 @@ DDTrace\flush();
bool(true)
array(1) {
["_dd.stack"]=>
&string(%d) "81a76578706c6f69749183a86c616e6775616765a3706870a26964a7736f6d65206964a66672616d65739284a46c696e6510a866756e6374696f6ea374776fa466696c65b77265706f72745f6261636b74726163655f30312e706870a269640084a46c696e6515a866756e6374696f6ea36f6e65a466696c65b77265706f72745f6261636b74726163655f30312e706870a2696401"
&string(%d) "81a76578706c6f69749183a86c616e6775616765a3706870a26964a7736f6d65206964a66672616d65739284a46c696e650da866756e6374696f6ea374776fa466696c65b77265706f72745f6261636b74726163655f30312e706870a269640084a46c696e6512a866756e6374696f6ea36f6e65a466696c65b77265706f72745f6261636b74726163655f30312e706870a2696401"
}
10 changes: 4 additions & 6 deletions appsec/tests/extension/report_backtrace_02.phpt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ extension=ddtrace.so
<?php
include __DIR__ . '/inc/ddtrace_version.php';

ddtrace_version_at_least('0.79.0');

use function datadog\appsec\testing\{report_exploit_backtrace, root_span_get_meta_struct};

function two($param01, $param02)
Expand Down Expand Up @@ -50,7 +48,7 @@ array(1) {
[0]=>
array(4) {
["line"]=>
int(15)
int(13)
["function"]=>
string(3) "two"
["file"]=>
Expand All @@ -61,7 +59,7 @@ array(1) {
[1]=>
array(4) {
["line"]=>
int(22)
int(20)
["function"]=>
string(3) "one"
["file"]=>
Expand All @@ -82,7 +80,7 @@ array(1) {
[0]=>
array(4) {
["line"]=>
int(15)
int(13)
["function"]=>
string(3) "two"
["file"]=>
Expand All @@ -93,7 +91,7 @@ array(1) {
[1]=>
array(4) {
["line"]=>
int(23)
int(21)
["function"]=>
string(3) "one"
["file"]=>
Expand Down
Loading

0 comments on commit 5668367

Please sign in to comment.