Skip to content

Commit

Permalink
More review changes
Browse files Browse the repository at this point in the history
  • Loading branch information
cataphract committed Oct 4, 2024
1 parent b424aab commit f1b0cb1
Show file tree
Hide file tree
Showing 13 changed files with 239 additions and 179 deletions.
4 changes: 2 additions & 2 deletions appsec/src/helper/json_helper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,9 @@ json_helper::get_field_of_type(
}

bool json_helper::parse_json(
const std::string &content, rapidjson::Document &output)
std::string_view content, rapidjson::Document &output)
{
if (output.Parse(content).HasParseError()) {
if (output.Parse(content.data(), content.size()).HasParseError()) {
SPDLOG_DEBUG("Invalid json: " + std::string(rapidjson::GetParseError_En(
output.GetParseError())));
return false;
Expand Down
2 changes: 1 addition & 1 deletion appsec/src/helper/json_helper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ std::optional<rapidjson::Value::ConstMemberIterator> get_field_of_type(
std::optional<rapidjson::Value::ConstMemberIterator> get_field_of_type(
rapidjson::Value::ConstValueIterator parent_field, std::string_view key,
rapidjson::Type type);
bool parse_json(const std::string &content, rapidjson::Document &output);
bool parse_json(std::string_view content, rapidjson::Document &output);
// NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
void merge_arrays(rapidjson::Value &destination, rapidjson::Value &source,
rapidjson::Value::AllocatorType &allocator);
Expand Down
17 changes: 2 additions & 15 deletions appsec/src/helper/remote_config/config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ config config::from_line(std::string_view line)
return {std::string{shm_path}, std::move(rc_path)};
}

std::string config::read() const
mapped_memory config::read() const
{
// open shared memory segment at rc_path:
const int fd = ::shm_open(shm_path.c_str(), O_RDONLY, 0);
Expand Down Expand Up @@ -100,19 +100,6 @@ std::string config::read() const
"Failed to map shared memory: " + std::string{strerror_ts(errno)});
}

auto unmap = defer{[shm_ptr, &shm_stat]() {
if (::munmap(shm_ptr, shm_stat.st_size) == -1) {
// NOLINTNEXTLINE(bugprone-lambda-function-name)
SPDLOG_WARN(
"Failed to unmap shared memory: {}", strerror_ts(errno));
}
}};

std::string result;
result.resize(shm_stat.st_size);

std::copy_n(static_cast<char *>(shm_ptr), shm_stat.st_size, result.begin());

return result;
return mapped_memory{shm_ptr, static_cast<std::size_t>(shm_stat.st_size)};
}
} // namespace dds::remote_config
44 changes: 43 additions & 1 deletion appsec/src/helper/remote_config/config.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,58 @@
#include <string_view>
#include <vector>

extern "C" {
#include <sys/mman.h>
}

namespace dds::remote_config {

class mapped_memory {
public:
mapped_memory(void *ptr, std::size_t size) : ptr_{ptr}, size_{size} {}
mapped_memory(const mapped_memory &) = delete;
mapped_memory(mapped_memory &&mm) noexcept : ptr_{mm.ptr_}, size_{mm.size_}
{
mm.ptr_ = nullptr;
mm.size_ = 0;
}
mapped_memory &operator=(const mapped_memory &) = delete;
mapped_memory &operator=(mapped_memory &&mm) noexcept
{
ptr_ = mm.ptr_;
size_ = mm.size_;
mm.ptr_ = nullptr;
mm.size_ = 0;
return *this;
}
~mapped_memory() noexcept
{
if (ptr_ != nullptr) {
if (::munmap(ptr_, size_) == -1) {
SPDLOG_WARN(
"Failed to unmap shared memory: {}", strerror_ts(errno));
};
}
}

operator std::string_view() const // NOLINT
{
return std::string_view{static_cast<char *>(ptr_), size_};
}

private:
void *ptr_;
std::size_t size_;
};

struct config {
// from a line provided by the RC config reader
static config from_line(std::string_view line);

std::string shm_path;
std::string rc_path;

[[nodiscard]] std::string read() const;
[[nodiscard]] mapped_memory read() const;

[[nodiscard]] product get_product() const;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,13 @@

void dds::remote_config::asm_features_listener::on_update(const config &config)
{
const std::string contents{config.read()};
rapidjson::Document serialized_doc;
if (!json_helper::parse_json(contents, serialized_doc)) {
throw error_applying_config("Invalid config contents");

{
const mapped_memory contents{config.read()};
if (!json_helper::parse_json(contents, serialized_doc)) {
throw error_applying_config("Invalid config contents");
}
}

auto asm_itr = json_helper::get_field_of_type(
Expand Down
47 changes: 47 additions & 0 deletions appsec/tests/extension/push_params_block_02.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
--TEST--
Push address gets blocked even when within a hook
--INI--
extension=ddtrace.so
datadog.appsec.enabled=1
--FILE--
<?php
use function datadog\appsec\testing\{rinit,rshutdown};
use function datadog\appsec\push_address;
use function datadog\appsec\testing\stop_for_debugger;

include __DIR__ . '/inc/mock_helper.php';

$helper = Helper::createInitedRun([
response_list(response_request_init([[['ok', []]]])),
response_list(response_request_exec([[['block', ['status_code' => '404', 'type' => 'html']]], ['{"found":"attack"}','{"another":"attack"}']])),
]);
rinit();

class SomeIntegration {
public function init()
{
DDTrace\install_hook("ltrim", self::hooked_function(), null);
}

private static function hooked_function()
{
return static function (DDTrace\HookData $hook) {
stop_for_debugger();
push_address("server.request.path_params", ["some" => "params", "more" => "parameters"]);
var_dump("This should be executed");
};
}
}

$integration = new SomeIntegration();
$integration->init();
echo PHP_EOL;
var_dump(ltrim(" Calling wrapped function"));
var_dump("THIS SHOULD NOT GET IN THE OUTPUT");

?>
--EXPECTHEADERS--
Status: 404 Not Found
Content-type: text/html; charset=UTF-8
--EXPECTF--
<!DOCTYPE html><html lang="en"><head><meta charset="UTF-8"><meta name="viewport" content="width=device-width,initial-scale=1"><title>You've been blocked</title><style>a,body,div,html,span{margin:0;padding:0;border:0;font-size:100%;font:inherit;vertical-align:baseline}body{background:-webkit-radial-gradient(26% 19%,circle,#fff,#f4f7f9);background:radial-gradient(circle at 26% 19%,#fff,#f4f7f9);display:-webkit-box;display:-ms-flexbox;display:flex;-webkit-box-pack:center;-ms-flex-pack:center;justify-content:center;-webkit-box-align:center;-ms-flex-align:center;align-items:center;-ms-flex-line-pack:center;align-content:center;width:100%;min-height:100vh;line-height:1;flex-direction:column}p{display:block}main{text-align:center;flex:1;display:-webkit-box;display:-ms-flexbox;display:flex;-webkit-box-pack:center;-ms-flex-pack:center;justify-content:center;-webkit-box-align:center;-ms-flex-align:center;align-items:center;-ms-flex-line-pack:center;align-content:center;flex-direction:column}p{font-size:18px;line-height:normal;color:#646464;font-family:sans-serif;font-weight:400}a{color:#4842b7}footer{width:100%;text-align:center}footer p{font-size:16px}</style></head><body><main><p>Sorry, you cannot access this page. Please contact the customer service team.</p></main><footer><p>Security provided by <a href="https://www.datadoghq.com/product/security-platform/application-security-monitoring/" target="_blank">Datadog</a></p></footer></body></html>
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,11 @@
namespace dds {

namespace mock = remote_config::mock;
auto &ASM_FEATURES = remote_config::known_products::ASM_FEATURES;

remote_config::config get_config_with_status(std::string status)
{
return mock::get_config(
ASM_FEATURES, "{\"asm\":{\"enabled\":" + status + "}}");
"ASM_FEATURES", "{\"asm\":{\"enabled\":" + status + "}}");
}

remote_config::config get_enabled_config(bool as_string = true)
Expand Down Expand Up @@ -130,7 +129,7 @@ TEST(RemoteConfigAsmFeaturesListener,
std::string error_message = "";
std::string expected_error_message = "Invalid config contents";
remote_config::config non_base_64_content_config =
mock::get_config(ASM_FEATURES, invalid_content);
mock::get_config("ASM_FEATURES", invalid_content);

try {
listener.on_update(non_base_64_content_config);
Expand All @@ -153,7 +152,7 @@ TEST(RemoteConfigAsmFeaturesListener,
remote_config::asm_features_listener listener(remote_config_service);
std::string invalid_content = "invalidJsonContent";
remote_config::config config =
mock::get_config(ASM_FEATURES, invalid_content);
mock::get_config("ASM_FEATURES", invalid_content);

try {
listener.on_update(config);
Expand All @@ -175,7 +174,7 @@ TEST(RemoteConfigAsmFeaturesListener, ListenerThrowsAnErrorWhenAsmKeyMissing)
auto remote_config_service = std::make_shared<service_config>();
remote_config::asm_features_listener listener(remote_config_service);
remote_config::config asm_key_missing =
mock::get_config(ASM_FEATURES, "{}");
mock::get_config("ASM_FEATURES", "{}");

try {
listener.on_update(asm_key_missing);
Expand All @@ -196,7 +195,7 @@ TEST(RemoteConfigAsmFeaturesListener, ListenerThrowsAnErrorWhenAsmIsNotValid)
auto remote_config_service = std::make_shared<service_config>();
remote_config::asm_features_listener listener(remote_config_service);
remote_config::config invalid_asm_key =
mock::get_config(ASM_FEATURES, "{ \"asm\": 123}");
mock::get_config("ASM_FEATURES", "{ \"asm\": 123}");

try {
listener.on_update(invalid_asm_key);
Expand All @@ -218,7 +217,7 @@ TEST(
auto remote_config_service = std::make_shared<service_config>();
remote_config::asm_features_listener listener(remote_config_service);
remote_config::config enabled_key_missing =
mock::get_config(ASM_FEATURES, "{ \"asm\": {}}");
mock::get_config("ASM_FEATURES", "{ \"asm\": {}}");

try {
listener.on_update(enabled_key_missing);
Expand All @@ -240,7 +239,7 @@ TEST(RemoteConfigAsmFeaturesListener,
auto remote_config_service = std::make_shared<service_config>();
remote_config::asm_features_listener listener(remote_config_service);
remote_config::config enabled_key_invalid =
mock::get_config(ASM_FEATURES, "{ \"asm\": { \"enabled\": 123}}");
mock::get_config("ASM_FEATURES", "{ \"asm\": { \"enabled\": 123}}");

try {
listener.on_update(enabled_key_invalid);
Expand Down
Loading

0 comments on commit f1b0cb1

Please sign in to comment.