Skip to content

Amend security_response_id being release before displaying it #3493

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Amend security_response_id being release before displaying it #3493

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3114270
Amend security_response_id being release before displaying it
estringana Nov 19, 2025
326d98f
Make emit error generic to use string arguments before releasing them
estringana Nov 21, 2025
ee63260
Fix issue when compile warning
estringana Nov 21, 2025
98d0a64
Fix issue
estringana Nov 25, 2025
4c9e791
Format
estringana Nov 25, 2025
7de55c1
Clean blocking parameter on shutdown
estringana Nov 26, 2025
0183e69
Addres PR comments
estringana Nov 28, 2025
193c190
Fix code
estringana Nov 28, 2025
2e9efdd
remove global block_params; centralize blocking logic
cataphract Nov 28, 2025
1947c09
Add comment
cataphract Nov 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions appsec/src/extension/commands/request_exec.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ static const dd_command_spec _spec = {
.config_features_cb = dd_command_process_config_features_unexpected,
};

dd_result dd_request_exec(
dd_conn *nonnull conn, zval *nonnull data, zend_string *nullable rasp_rule)
dd_result dd_request_exec(dd_conn *nonnull conn, zval *nonnull data,
zend_string *nullable rasp_rule, struct block_params *nonnull block_params)
{
if (Z_TYPE_P(data) != IS_ARRAY) {
mlog(dd_log_debug, "Invalid data provided to command request_exec, "
Expand All @@ -41,7 +41,11 @@ dd_result dd_request_exec(

struct ctx ctx = {.rasp_rule = rasp_rule, .data = data};

return dd_command_exec_req_info(conn, &_spec, &ctx.req_info);
dd_result res = dd_command_exec_req_info(conn, &_spec, &ctx.req_info);

memcpy(block_params, &ctx.req_info.block_params, sizeof *block_params);

return res;
}

static dd_result _pack_command(mpack_writer_t *nonnull w, void *nonnull _ctx)
Expand Down
8 changes: 5 additions & 3 deletions appsec/src/extension/commands/request_exec.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
// (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc.
#pragma once

#include "../network.h"
#include <SAPI.h>
#include <php.h>

dd_result dd_request_exec(
dd_conn *nonnull conn, zval *nonnull data, zend_string *nullable rasp_rule);
#include "../network.h"
#include "../request_abort.h"

dd_result dd_request_exec(dd_conn *nonnull conn, zval *nonnull data,
zend_string *nullable rasp_rule, struct block_params *nonnull block_params);
2 changes: 2 additions & 0 deletions appsec/src/extension/commands_ctx.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@

#include <php.h>
#include "attributes.h"
#include "request_abort.h"

struct req_info {
const char *nullable command_name; // for logging
zend_object *nullable root_span;
zend_string *nullable client_ip;
struct block_params block_params; // out
};
66 changes: 59 additions & 7 deletions appsec/src/extension/commands_helpers.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,17 @@ static inline ATTR_WARN_UNUSED mpack_error_t _imsg_destroy(
dd_imsg *nonnull imsg);
static void _imsg_cleanup(dd_imsg *nullable *imsg);

static void _set_redirect_code_and_location(
struct block_params *nonnull block_params,
// NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
int code, zend_string *nullable location,
zend_string *nullable security_response_id);

static void _set_block_code_and_type(struct block_params *nonnull block_params,
// NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
int code, dd_response_type type,
zend_string *nullable security_response_id);

static dd_result _dd_command_exec(dd_conn *nonnull conn,
const dd_command_spec *nonnull spec, void *unspecnull ctx)
{
Expand Down Expand Up @@ -299,7 +310,8 @@ static void _imsg_cleanup(dd_imsg *nullable *imsg)
static void _add_appsec_span_data_frag(mpack_node_t node);
static void _set_appsec_span_data(mpack_node_t node);

static void _command_process_block_parameters(mpack_node_t root)
static void _command_process_block_parameters(
struct block_params *nonnull block_params, mpack_node_t root)
{
int status_code = DEFAULT_BLOCKING_RESPONSE_CODE;
dd_response_type type = DEFAULT_RESPONSE_TYPE;
Expand Down Expand Up @@ -365,10 +377,12 @@ static void _command_process_block_parameters(mpack_node_t root)
"Blocking parameters: status_code=%d, type=%d, security_response_id=%s",
status_code, type,
security_response_id ? ZSTR_VAL(security_response_id) : "NULL");
dd_set_block_code_and_type(status_code, type, security_response_id);
_set_block_code_and_type(
block_params, status_code, type, security_response_id);
}

static void _command_process_redirect_parameters(mpack_node_t root)
static void _command_process_redirect_parameters(
struct block_params *nonnull block_params, mpack_node_t root)
{
int status_code = 0;
zend_string *location = NULL;
Expand Down Expand Up @@ -423,9 +437,46 @@ static void _command_process_redirect_parameters(mpack_node_t root)
"security_response_id=%s",
status_code, location ? ZSTR_VAL(location) : "NULL",
security_response_id ? ZSTR_VAL(security_response_id) : "NULL");
dd_set_redirect_code_and_location(
status_code, location, security_response_id);

_set_redirect_code_and_location(
block_params, status_code, location, security_response_id);
}

static void _set_block_code_and_type(struct block_params *nonnull block_params,
// NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
int code, dd_response_type type, zend_string *nullable security_response_id)
{
dd_response_type _type = type;
// Account for lack of enum type safety
switch (type) {
case response_type_auto:
case response_type_html:
case response_type_json:
_type = type;
break;
default:
_type = response_type_auto;
break;
}

block_params->security_response_id = security_response_id;
block_params->response_type = _type;
block_params->response_code = code;
block_params->redirection_location = NULL;
}

static void _set_redirect_code_and_location(
struct block_params *nonnull block_params,
// NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
int code, zend_string *nullable location,
zend_string *nullable security_response_id)
{
block_params->security_response_id = security_response_id;
block_params->response_type = response_type_auto;
block_params->response_code = code;
block_params->redirection_location = location;
}

static void _command_process_stack_trace_parameters(mpack_node_t root)
{
size_t count = mpack_node_map_count(root);
Expand Down Expand Up @@ -540,13 +591,14 @@ static dd_result _command_process_actions(
if (dd_mpack_node_lstr_eq(verdict, "block") && res != dd_should_block &&
res != dd_should_redirect) { // Redirect take over block
res = dd_should_block;
_command_process_block_parameters(mpack_node_array_at(action, 1));
_command_process_block_parameters(
&ctx->block_params, mpack_node_array_at(action, 1));
dd_tags_add_blocked();
} else if (dd_mpack_node_lstr_eq(verdict, "redirect") &&
res != dd_should_redirect) {
res = dd_should_redirect;
_command_process_redirect_parameters(
mpack_node_array_at(action, 1));
&ctx->block_params, mpack_node_array_at(action, 1));
dd_tags_add_blocked();
} else if (dd_mpack_node_lstr_eq(verdict, "record") &&
res == dd_success) {
Expand Down
29 changes: 11 additions & 18 deletions appsec/src/extension/ddappsec.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,6 @@ static PHP_RINIT_FUNCTION(ddappsec)
DDAPPSEC_G(skip_rshutdown) = false;
dd_msgpack_helpers_rinit();
dd_trace_rinit();
dd_request_abort_rinit();

// Waf calls happen here. Not many rinits should go after this line.
dd_req_lifecycle_rinit(false);
Expand All @@ -288,7 +287,7 @@ static PHP_RINIT_FUNCTION(ddappsec)
if (get_global_DD_APPSEC_TESTING_ABORT_RINIT()) {
const char *pt = SG(request_info).path_translated;
if (pt && !strstr(pt, "skip.php")) {
dd_request_abort_static_page();
dd_request_abort_static_page(&(struct block_params){0});
}
}
}
Expand Down Expand Up @@ -475,11 +474,13 @@ static PHP_FUNCTION(datadog_appsec_testing_request_exec)
RETURN_FALSE;
}

if (dd_request_exec(conn, data, false) != dd_success) {
RETURN_FALSE;
struct block_params block_params = {0};
if (dd_request_exec(conn, data, false, &block_params) != dd_success) {
RETVAL_FALSE;
} else {
RETVAL_TRUE;
}

RETURN_TRUE;
dd_request_abort_destroy_block_params(&block_params);
}

static PHP_FUNCTION(datadog_appsec_push_addresses)
Expand Down Expand Up @@ -517,7 +518,8 @@ static PHP_FUNCTION(datadog_appsec_push_addresses)
return;
}

dd_result res = dd_request_exec(conn, addresses, rasp_rule);
struct block_params block_params = {0};
dd_result res = dd_request_exec(conn, addresses, rasp_rule, &block_params);

if (rasp_rule && ZSTR_LEN(rasp_rule) > 0) {
clock_gettime(CLOCK_MONOTONIC_RAW, &end);
Expand All @@ -532,17 +534,8 @@ static PHP_FUNCTION(datadog_appsec_push_addresses)
}
}

if (dd_req_is_user_req()) {
if (res == dd_should_block || res == dd_should_redirect) {
dd_req_call_blocking_function(res);
}
} else {
if (res == dd_should_block) {
dd_request_abort_static_page();
} else if (res == dd_should_redirect) {
dd_request_abort_redirect();
}
}
dd_req_lifecycle_abort(REQUEST_STAGE_REQUEST_END, res, &block_params);
dd_request_abort_destroy_block_params(&block_params);
}

ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(
Expand Down
Loading
Loading