From 2073dc8ba3457ced4b77e9d10d402f433cd88a88 Mon Sep 17 00:00:00 2001 From: Pawel Filipczak Date: Wed, 7 Feb 2024 11:21:59 +0100 Subject: [PATCH 1/4] Sanitize cli arguments --- agent/native/ext/MemoryTracker.cpp | 6 +++++- .../AutoInstrument/TransactionForExtensionRequest.php | 8 ++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/agent/native/ext/MemoryTracker.cpp b/agent/native/ext/MemoryTracker.cpp index edd08bd59..c52f4435d 100644 --- a/agent/native/ext/MemoryTracker.cpp +++ b/agent/native/ext/MemoryTracker.cpp @@ -241,6 +241,10 @@ void removeFromTrackedAllocatedBlocks( IntrusiveDoublyLinkedList* allocatedBlocks, size_t* possibleActuallyRequestedSize ) { + if (!allocatedBlock) { + return; + } + EmbeddedTrackingDataHeader* trackingDataHeader = allocatedBlockToTrackingData( allocatedBlock, originallyRequestedSize ); verifyMagic( "prefix", trackingDataHeader->prefixMagic, prefixMagicExpectedValue ); @@ -275,7 +279,7 @@ void memoryTrackerBeforeFree( IntrusiveDoublyLinkedList* allocatedBlocks = isPersistent ? &memTracker->allocatedPersistentBlocks : &memTracker->allocatedRequestScopedBlocks; ELASTIC_APM_ASSERT( *allocated >= originallyRequestedSize - , "Attempting to free more %s memory than allocated. Allocated: %" PRIu64 ". Attempting to free: %" PRIu64 + , "Attempting to free more %s memory than allocated. Allocated: %" PRIu64 ". Attempting to free: %" PRIu64 , allocType( isPersistent ), *allocated, (UInt64)originallyRequestedSize ); *possibleActuallyRequestedSize = originallyRequestedSize; diff --git a/agent/php/ElasticApm/Impl/AutoInstrument/TransactionForExtensionRequest.php b/agent/php/ElasticApm/Impl/AutoInstrument/TransactionForExtensionRequest.php index 95bd3376b..788a7dd2e 100644 --- a/agent/php/ElasticApm/Impl/AutoInstrument/TransactionForExtensionRequest.php +++ b/agent/php/ElasticApm/Impl/AutoInstrument/TransactionForExtensionRequest.php @@ -420,6 +420,10 @@ private static function isCliScript(): bool return PHP_SAPI === 'cli'; } + private function sanitizeCliName(string $name): string { + return preg_replace('/[^a-zA-Z0-9.:_\-]/', '_', $name); + } + private function discoverCliName(): string { global $argc, $argv; @@ -452,7 +456,7 @@ private function discoverCliName(): string 'Using CLI script name as transaction name', ['cliScriptName' => $cliScriptName, 'argc' => $argc, 'argv' => $argv] ); - return $cliScriptName; + return $this->sanitizeCliName($cliScriptName); } $txName = $cliScriptName . ' ' . $argv[1]; @@ -462,7 +466,7 @@ private function discoverCliName(): string . ' - including the first argument in transaction name', ['txName' => $txName, 'argc' => $argc, 'argv' => $argv] ); - return $txName; + return $this->sanitizeCliName($txName); } /** From 56a4040b516a5182205886c4f21108061e7fb1b8 Mon Sep 17 00:00:00 2001 From: Pawel Filipczak Date: Wed, 7 Feb 2024 11:29:48 +0100 Subject: [PATCH 2/4] fixed static checks --- .../Impl/AutoInstrument/TransactionForExtensionRequest.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/agent/php/ElasticApm/Impl/AutoInstrument/TransactionForExtensionRequest.php b/agent/php/ElasticApm/Impl/AutoInstrument/TransactionForExtensionRequest.php index 788a7dd2e..ed235bb8b 100644 --- a/agent/php/ElasticApm/Impl/AutoInstrument/TransactionForExtensionRequest.php +++ b/agent/php/ElasticApm/Impl/AutoInstrument/TransactionForExtensionRequest.php @@ -420,8 +420,9 @@ private static function isCliScript(): bool return PHP_SAPI === 'cli'; } - private function sanitizeCliName(string $name): string { - return preg_replace('/[^a-zA-Z0-9.:_\-]/', '_', $name); + private function sanitizeCliName(string $name): string + { + return preg_replace('/[^a-zA-Z0-9.:_\-]/', '_', $name) ?: ''; } private function discoverCliName(): string From c4180d0911482fadf2bf053eba4bb535a4e523b6 Mon Sep 17 00:00:00 2001 From: Pawel Filipczak Date: Wed, 7 Feb 2024 11:33:46 +0100 Subject: [PATCH 3/4] make func static --- .../AutoInstrument/TransactionForExtensionRequest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/agent/php/ElasticApm/Impl/AutoInstrument/TransactionForExtensionRequest.php b/agent/php/ElasticApm/Impl/AutoInstrument/TransactionForExtensionRequest.php index ed235bb8b..a7e409706 100644 --- a/agent/php/ElasticApm/Impl/AutoInstrument/TransactionForExtensionRequest.php +++ b/agent/php/ElasticApm/Impl/AutoInstrument/TransactionForExtensionRequest.php @@ -420,9 +420,9 @@ private static function isCliScript(): bool return PHP_SAPI === 'cli'; } - private function sanitizeCliName(string $name): string + private static function sanitizeCliName(string $name): string { - return preg_replace('/[^a-zA-Z0-9.:_\-]/', '_', $name) ?: ''; + return preg_replace('/[^a-zA-Z0-9.:_\-]/', '_', $name) ?: ' '; } private function discoverCliName(): string @@ -457,7 +457,7 @@ private function discoverCliName(): string 'Using CLI script name as transaction name', ['cliScriptName' => $cliScriptName, 'argc' => $argc, 'argv' => $argv] ); - return $this->sanitizeCliName($cliScriptName); + return self::sanitizeCliName($cliScriptName); } $txName = $cliScriptName . ' ' . $argv[1]; @@ -467,7 +467,7 @@ private function discoverCliName(): string . ' - including the first argument in transaction name', ['txName' => $txName, 'argc' => $argc, 'argv' => $argv] ); - return $this->sanitizeCliName($txName); + return self::sanitizeCliName($txName); } /** From 8865f09f97aad4aeca77af39192c20992876661a Mon Sep 17 00:00:00 2001 From: Pawel Filipczak Date: Wed, 7 Feb 2024 11:36:36 +0100 Subject: [PATCH 4/4] make it earlier --- .../AutoInstrument/TransactionForExtensionRequest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/agent/php/ElasticApm/Impl/AutoInstrument/TransactionForExtensionRequest.php b/agent/php/ElasticApm/Impl/AutoInstrument/TransactionForExtensionRequest.php index a7e409706..85102126b 100644 --- a/agent/php/ElasticApm/Impl/AutoInstrument/TransactionForExtensionRequest.php +++ b/agent/php/ElasticApm/Impl/AutoInstrument/TransactionForExtensionRequest.php @@ -446,7 +446,7 @@ private function discoverCliName(): string return self::DEFAULT_NAME; } - $cliScriptName = basename($argv[0]); + $cliScriptName = self::sanitizeCliName(basename($argv[0])); if ( ($argc < 2) || (count($argv) < 2) @@ -457,17 +457,17 @@ private function discoverCliName(): string 'Using CLI script name as transaction name', ['cliScriptName' => $cliScriptName, 'argc' => $argc, 'argv' => $argv] ); - return self::sanitizeCliName($cliScriptName); + return $cliScriptName; } - $txName = $cliScriptName . ' ' . $argv[1]; + $txName = $cliScriptName . ' ' . self::sanitizeCliName($argv[1]); ($loggerProxy = $this->logger->ifDebugLevelEnabled(__LINE__, __FUNCTION__)) && $loggerProxy->log( 'CLI script is Laravel ' . self::LARAVEL_ARTISAN_COMMAND_SCRIPT . ' command with arguments' . ' - including the first argument in transaction name', ['txName' => $txName, 'argc' => $argc, 'argv' => $argv] ); - return self::sanitizeCliName($txName); + return $txName; } /**