Skip to content

Commit

Permalink
feat(*): Add buildRequestRawBody helper (#133)
Browse files Browse the repository at this point in the history
* feat(bouncer): Add buildRequestrawBody helper

* style(*): Pass through code format tools

* feat(bouncer): Avoid infinite loop in buildRequestrawBody

* ci(test): Exclude some test for php < 7.4

* feat(bouncer): Improve infinite loop security log message

* feat(*): Prepare release 3.2.0

* feat(bouncer): Improve boundary extraction

* style(*): Remove trailing commas for php 7
  • Loading branch information
julienloizelet authored Oct 23, 2024
1 parent 0b4c021 commit 68e0c83
Show file tree
Hide file tree
Showing 13 changed files with 839 additions and 15 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/coding-standards.yml
Original file line number Diff line number Diff line change
Expand Up @@ -109,5 +109,5 @@ jobs:
if: github.event.inputs.coverage_report == 'true'
run: |
ddev xdebug
ddev exec XDEBUG_MODE=coverage BOUNCER_KEY=${{ env.BOUNCER_KEY }} AGENT_TLS_PATH=/var/www/html/cfssl LAPI_URL=https://crowdsec:8080 MEMCACHED_DSN=memcached://memcached:11211 REDIS_DSN=redis://redis:6379 /usr/bin/php ./${{env.EXTENSION_PATH}}/tools/coding-standards/vendor/bin/phpunit --configuration ./${{env.EXTENSION_PATH}}/tools/coding-standards/phpunit/phpunit.xml --coverage-text=./${{env.EXTENSION_PATH}}/coding-standards/phpunit/code-coverage/report.txt
ddev exec XDEBUG_MODE=coverage BOUNCER_KEY=${{ env.BOUNCER_KEY }} APPSEC_URL=http://crowdsec:7422 AGENT_TLS_PATH=/var/www/html/cfssl LAPI_URL=https://crowdsec:8080 MEMCACHED_DSN=memcached://memcached:11211 REDIS_DSN=redis://redis:6379 /usr/bin/php ./${{env.EXTENSION_PATH}}/tools/coding-standards/vendor/bin/phpunit --configuration ./${{env.EXTENSION_PATH}}/tools/coding-standards/phpunit/phpunit.xml --coverage-text=./${{env.EXTENSION_PATH}}/coding-standards/phpunit/code-coverage/report.txt
cat ${{env.EXTENSION_PATH}}/coding-standards/phpunit/code-coverage/report.txt
7 changes: 6 additions & 1 deletion .github/workflows/test-suite.yml
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,14 @@ jobs:
run: |
ddev composer update --working-dir ./${{env.EXTENSION_PATH}}
- name: Set excluded groups
id: set-excluded-groups
if: contains(fromJson('["7.2","7.3"]'),matrix.php-version)
run: echo "exclude_group=$(echo --exclude-group up-to-php74 )" >> $GITHUB_OUTPUT

- name: Run "Unit Tests"
run: |
ddev exec /usr/bin/php ./${{env.EXTENSION_PATH}}/vendor/bin/phpunit --testdox --colors --exclude-group ignore ./${{env.EXTENSION_PATH}}/tests/Unit
ddev exec /usr/bin/php ./${{env.EXTENSION_PATH}}/vendor/bin/phpunit --testdox ${{ steps.set-excluded-groups.outputs.exclude_group }} ./${{env.EXTENSION_PATH}}/tests/Unit
- name: Prepare PHP Integration tests
run: |
Expand Down
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,16 @@ As far as possible, we try to adhere to [Symfony guidelines](https://symfony.com

---

## [3.2.0](https://github.com/crowdsecurity/php-cs-bouncer/releases/tag/v3.2.0) - 2024-10-23
[_Compare with previous release_](https://github.com/crowdsecurity/php-cs-bouncer/compare/v3.1.0...v3.2.0)


### Added

- Add protected `buildRequestRawBody` helper method to `AbstractBouncer` class

---

## [3.1.0](https://github.com/crowdsecurity/php-cs-bouncer/releases/tag/v3.1.0) - 2024-10-18
[_Compare with previous release_](https://github.com/crowdsecurity/php-cs-bouncer/compare/v3.0.0...v3.1.0)

Expand Down
4 changes: 1 addition & 3 deletions docs/DEVELOPER.md
Original file line number Diff line number Diff line change
Expand Up @@ -286,9 +286,7 @@ ddev xdebug

To generate a html report, you can run:
```bash
ddev exec XDEBUG_MODE=coverage APPSEC_URL=http://crowdsec:7422 BOUNCER_KEY=your-bouncer-key
AGENT_TLS_PATH=/var/www/html/cfssl LAPI_URL=https://crowdsec:8080
REDIS_DSN=redis://redis:6379 MEMCACHED_DSN=memcached://memcached:11211 /usr/bin/php ./my-code/crowdsec-bouncer-lib/tools/coding-standards/vendor/bin/phpunit --configuration ./my-code/crowdsec-bouncer-lib/tools/coding-standards/phpunit/phpunit.xml
ddev exec XDEBUG_MODE=coverage APPSEC_URL=http://crowdsec:7422 BOUNCER_KEY=your-bouncer-key AGENT_TLS_PATH=/var/www/html/cfssl LAPI_URL=https://crowdsec:8080 REDIS_DSN=redis://redis:6379 MEMCACHED_DSN=memcached://memcached:11211 /usr/bin/php ./my-code/crowdsec-bouncer-lib/tools/coding-standards/vendor/bin/phpunit --configuration ./my-code/crowdsec-bouncer-lib/tools/coding-standards/phpunit/phpunit.xml

```

Expand Down
32 changes: 32 additions & 0 deletions docs/USER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,38 @@ class MyCustomBouncer extends AbstractBouncer
{
// Your implementation
}

/**
* Get current request headers
*/
public function getRequestHeaders(): array
{
// Your implementation
}

/**
* Get the raw body of the current request
*/
public function getRequestRawBody(): string
{
// Your implementation
}

/**
* Get the host of the current request
*/
public function getRequestHost() : string
{
// Your implementation
}

/**
* Get the user agent of the current request
*/
public function getRequestUserAgent() : string
{
// Your implementation
}

}
```
Expand Down
37 changes: 37 additions & 0 deletions src/AbstractBouncer.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@
*/
abstract class AbstractBouncer
{
use Helper;

/** @var array */
protected $configs = [];
/** @var LoggerInterface */
Expand Down Expand Up @@ -326,6 +328,41 @@ public function testCacheConnection(): void
}
}

/**
* Method based on superglobals to retrieve the raw body of the request.
* If the body is too big (greater than the "appsec_max_body_size_kb" configuration),
* it will be truncated to the maximum size + 1 kB.
* In case of error, an empty string is returned.
*
* @param resource $stream The stream to read the body from
*
* @see https://www.php.net/manual/en/language.variables.superglobals.php
*/
protected function buildRequestRawBody($stream): string
{
if (!is_resource($stream)) {
$this->logger->error('Invalid stream resource', [
'type' => 'BUILD_RAW_BODY',
]);

return '';
}
$maxBodySize = $this->getRemediationEngine()->getConfig('appsec_max_body_size_kb') ??
Constants::APPSEC_DEFAULT_MAX_BODY_SIZE;

try {
return $this->buildRawBodyFromSuperglobals($maxBodySize, $stream, $_SERVER, $_POST, $_FILES);
} catch (BouncerException $e) {
$this->logger->error('Error while building raw body', [
'type' => 'BUILD_RAW_BODY',
'message' => $e->getMessage(),
'code' => $e->getCode(),
]);

return '';
}
}

/**
* Returns a default "CrowdSec 403" HTML template.
* The input $config should match the TemplateConfiguration input format.
Expand Down
2 changes: 1 addition & 1 deletion src/Constants.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class Constants extends RemConstants
/** @var string Path for html templates folder (e.g. ban and captcha wall) */
public const TEMPLATES_DIR = __DIR__ . '/templates';
/** @var string The last version of this library */
public const VERSION = 'v3.1.0';
public const VERSION = 'v3.2.0';
/** @var string The "disabled" x-forwarded-for setting */
public const X_FORWARDED_DISABLED = 'no_forward';
}
223 changes: 223 additions & 0 deletions src/Helper.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,223 @@
<?php

declare(strict_types=1);

namespace CrowdSecBouncer;

/**
* Helper trait for Bouncer.
*
* @author CrowdSec team
*
* @see https://crowdsec.net CrowdSec Official Website
*
* @copyright Copyright (c) 2021+ CrowdSec
* @license MIT License
*/
trait Helper
{
/**
* Build the raw body from superglobals.
*
* @param int $maxBodySize the maximum body size in KB
* @param resource $stream The stream to read
* @param array $serverData the $_SERVER superglobal
* @param array $postData the $_POST superglobal
* @param array $filesData the $_FILES superglobal
*
* @return string the raw body
*
* @throws BouncerException
*/
private function buildRawBodyFromSuperglobals(
int $maxBodySize,
$stream,
array $serverData = [], // $_SERVER
array $postData = [], // $_POST
array $filesData = [] // $_FILES
): string {
$contentType = $serverData['CONTENT_TYPE'] ?? '';
// The threshold is the maximum body size converted in bytes + 1
$sizeThreshold = ($maxBodySize * 1024) + 1;

if (false !== strpos($contentType, 'multipart/')) {
return $this->getMultipartRawBody($contentType, $sizeThreshold, $postData, $filesData);
}

return $this->getRawInput($sizeThreshold, $stream);
}

private function appendFileData(
array $fileArray,
?int $index,
string $fileKey,
string $boundary,
int $threshold,
int &$currentSize
): string {
$fileName = is_array($fileArray['name']) ? $fileArray['name'][$index] : $fileArray['name'];
$fileTmpName = is_array($fileArray['tmp_name']) ? $fileArray['tmp_name'][$index] : $fileArray['tmp_name'];
$fileType = is_array($fileArray['type']) ? $fileArray['type'][$index] : $fileArray['type'];

$headerPart = '--' . $boundary . "\r\n";
$headerPart .= "Content-Disposition: form-data; name=\"$fileKey\"; filename=\"$fileName\"\r\n";
$headerPart .= "Content-Type: $fileType\r\n\r\n";

$currentSize += strlen($headerPart);
if ($currentSize >= $threshold) {
return substr($headerPart, 0, $threshold - ($currentSize - strlen($headerPart)));
}

$remainingSize = $threshold - $currentSize;
$fileStream = fopen($fileTmpName, 'rb');
$fileContent = $this->readStream($fileStream, $remainingSize);
// Add 2 bytes for the \r\n at the end of the file content
$currentSize += strlen($fileContent) + 2;

return $headerPart . $fileContent . "\r\n";
}

private function buildFormData(string $boundary, string $key, string $value): string
{
return '--' . $boundary . "\r\n" .
"Content-Disposition: form-data; name=\"$key\"\r\n\r\n" .
"$value\r\n";
}

/**
* Extract the boundary from the Content-Type.
*
* Regex breakdown:
* /boundary="?([^;"]+)"?/i
*
* - boundary= : Matches the literal string 'boundary=' which indicates the start of the boundary parameter.
* - "? : Matches an optional double quote that may surround the boundary value.
* - ([^;"]+) : Captures one or more characters that are not a semicolon (;) or a double quote (") into a group.
* This ensures the boundary is extracted accurately, stopping at a semicolon if present,
* and avoiding the inclusion of quotes in the captured value.
* - "? : Matches an optional closing double quote (if the boundary is quoted).
* - i : Case-insensitive flag to handle 'boundary=' in any case (e.g., 'Boundary=' or 'BOUNDARY=').
*
* @throws BouncerException
*/
private function extractBoundary(string $contentType): string
{
if (preg_match('/boundary="?([^;"]+)"?/i', $contentType, $matches)) {
return trim($matches[1]);
}
throw new BouncerException("Failed to extract boundary from Content-Type: ($contentType)");
}

/**
* Return the raw body for multipart requests.
* This method will read the raw body up to the specified threshold.
* If the body is too large, it will return a truncated version of the body up to the threshold.
*
* @throws BouncerException
*/
private function getMultipartRawBody(
string $contentType,
int $threshold,
array $postData,
array $filesData
): string {
try {
$boundary = $this->extractBoundary($contentType);
// Instead of concatenating strings, we will use an array to store the parts
// and then join them with implode at the end to avoid performance issues.
$parts = [];
$currentSize = 0;

foreach ($postData as $key => $value) {
$formData = $this->buildFormData($boundary, $key, $value);
$currentSize += strlen($formData);
if ($currentSize >= $threshold) {
return substr(implode('', $parts) . $formData, 0, $threshold);
}

$parts[] = $formData;
}

foreach ($filesData as $fileKey => $fileArray) {
$fileNames = is_array($fileArray['name']) ? $fileArray['name'] : [$fileArray['name']];
foreach ($fileNames as $index => $fileName) {
$remainingSize = $threshold - $currentSize;
$fileData =
$this->appendFileData($fileArray, $index, $fileKey, $boundary, $remainingSize, $currentSize);
if ($currentSize >= $threshold) {
return substr(implode('', $parts) . $fileData, 0, $threshold);
}
$parts[] = $fileData;
}
}

$endBoundary = '--' . $boundary . "--\r\n";
$currentSize += strlen($endBoundary);

if ($currentSize >= $threshold) {
return substr(implode('', $parts) . $endBoundary, 0, $threshold);
}

$parts[] = $endBoundary;

return implode('', $parts);
} catch (\Throwable $e) {
throw new BouncerException('Failed to read multipart raw body: ' . $e->getMessage());
}
}

private function getRawInput(int $threshold, $stream): string
{
return $this->readStream($stream, $threshold);
}

/**
* Read the stream up to the specified threshold.
*
* @param resource $stream The stream to read
* @param int $threshold The maximum number of bytes to read
*
* @throws BouncerException
*/
private function readStream($stream, int $threshold): string
{
if (!is_resource($stream)) {
throw new BouncerException('Stream is not a valid resource');
}
$buffer = '';
$chunkSize = 8192;
$bytesRead = 0;
// We make sure there won't be infinite loop
$maxLoops = (int) ceil($threshold / $chunkSize);
$loopCount = -1;

try {
while (!feof($stream) && $bytesRead < $threshold) {
++$loopCount;
if ($loopCount >= $maxLoops) {
throw new BouncerException("Too many loops ($loopCount) while reading stream");
}
$remainingSize = $threshold - $bytesRead;
$readLength = min($chunkSize, $remainingSize);

$data = fread($stream, $readLength);
if (false === $data) {
throw new BouncerException('Failed to read chunk from stream');
}

$buffer .= $data;
$bytesRead += strlen($data);

if ($bytesRead >= $threshold) {
break;
}
}

return $buffer;
} catch (\Throwable $e) {
throw new BouncerException('Failed to read stream: ' . $e->getMessage());
} finally {
fclose($stream);
}
}
}
Loading

0 comments on commit 68e0c83

Please sign in to comment.