Fix #185: Abgeschnittene PayPal-NVP-Antworten im Abrufprotokoll
This commit is contained in:
parent
122521f0f7
commit
4cde18294a
3 changed files with 143 additions and 4 deletions
|
|
@ -2,6 +2,13 @@
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
## 2026-08-18 – PayPal-Abruf: abgeschnittene NVP-Antworten im Protokoll (#185)
|
||||||
|
|
||||||
|
- `PayPalNvpFetchService::externalIdExists()` dedupliziert bereits über `external_id` = die PayPal-Transaktionsnummer (`L_TRANSACTIONID`), nicht über Datum/Betrag/Text – eine nachträglich korrigierte Buchung wird also schon heute nicht doppelt importiert. Kein Änderungsbedarf an diesem Teil des Issues.
|
||||||
|
- `nvpPost()` behandelte einen abgebrochenen/unvollständigen HTTP-Transfer bisher wie einen normalen Fehler und protokollierte nur den HTTP-Status – teils `0` ohne weitere Erklärung, ohne dass sichtbar wurde, dass die Antwort abgeschnitten war.
|
||||||
|
- **`src/Services/PayPalNvpFetchService.php`:** `nvpPost()` wertet jetzt zusätzlich `curl_errno()`/`curl_error()` sowie `CURLINFO_CONTENT_LENGTH_DOWNLOAD` aus. Neue Methode `isTruncatedTransferError()` erkennt abgebrochene Transfers (u.a. `CURLE_PARTIAL_FILE`, `CURLE_GOT_NOTHING`, `CURLE_RECV_ERROR`, sowie die Fehlermeldungen „transfer closed“/„Empty reply“) und schreibt dafür einen erklärenden Hinweis ins Abrufprotokoll statt der bisherigen stillen HTTP-Status-Zeile. Neue Methode `isLengthMismatch()` erkennt zusätzlich den Fall, dass trotz HTTP 200 weniger Bytes ankamen als im `Content-Length`-Header angekündigt, und meldet das ebenfalls als Hinweis.
|
||||||
|
- **`tests/91_PayPalNvpAbgeschnitteneAntwortTest.php`** (neu): Deckt `isTruncatedTransferError()` (bekannte cURL-Fehlercodes, Fehlermeldungstexte, normale Fehler ohne Falsch-Alarm) und `isLengthMismatch()` (kürzere Antwort, übereinstimmende Länge, fehlende Content-Length) über Reflection auf die privaten statischen Methoden ab – ohne echte PayPal-Verbindung.
|
||||||
|
|
||||||
## 2026-08-12 – SSRF-Schutz für SMTP-Einstellungen (#183)
|
## 2026-08-12 – SSRF-Schutz für SMTP-Einstellungen (#183)
|
||||||
|
|
||||||
- `mail_smtp_host`/`mail_smtp_port` wurden ungeprüft aus `Setting::get()` an `SmtpMailer::connect()` durchgereicht, das per `stream_socket_client()` eine TCP-/TLS-Verbindung zu genau diesem Ziel aufbaut – anders als `fints_url` (#125) und `nextcloud_url` (#127) ohne jede Prüfung auf private/interne Adressen. Über `POST /settings/mail-test` konnte ein authentifizierter Nutzer damit interne Netzwerk-/Portscans auslösen (Blind-SSRF-Oracle, CWE-918), da unterschiedliche `RuntimeException`-Nachrichten offene/geschlossene/gefilterte Ports bzw. Fremddienste erkennbar machten.
|
- `mail_smtp_host`/`mail_smtp_port` wurden ungeprüft aus `Setting::get()` an `SmtpMailer::connect()` durchgereicht, das per `stream_socket_client()` eine TCP-/TLS-Verbindung zu genau diesem Ziel aufbaut – anders als `fints_url` (#125) und `nextcloud_url` (#127) ohne jede Prüfung auf private/interne Adressen. Über `POST /settings/mail-test` konnte ein authentifizierter Nutzer damit interne Netzwerk-/Portscans auslösen (Blind-SSRF-Oracle, CWE-918), da unterschiedliche `RuntimeException`-Nachrichten offene/geschlossene/gefilterte Ports bzw. Fremddienste erkennbar machten.
|
||||||
|
|
|
||||||
|
|
@ -276,19 +276,55 @@ class PayPalNvpFetchService
|
||||||
CURLOPT_HTTPHEADER => ['Content-Type: application/x-www-form-urlencoded'],
|
CURLOPT_HTTPHEADER => ['Content-Type: application/x-www-form-urlencoded'],
|
||||||
CURLOPT_TIMEOUT => 30,
|
CURLOPT_TIMEOUT => 30,
|
||||||
]);
|
]);
|
||||||
$response = curl_exec($ch);
|
$response = curl_exec($ch);
|
||||||
$code = curl_getinfo($ch, CURLINFO_HTTP_CODE);
|
$code = curl_getinfo($ch, CURLINFO_HTTP_CODE);
|
||||||
|
$errNo = curl_errno($ch);
|
||||||
|
$errMsg = curl_error($ch);
|
||||||
|
$declaredLen = (float)curl_getinfo($ch, CURLINFO_CONTENT_LENGTH_DOWNLOAD);
|
||||||
curl_close($ch);
|
curl_close($ch);
|
||||||
|
|
||||||
if ($code !== 200 || !$response) {
|
if ($response === false) {
|
||||||
|
if (self::isTruncatedTransferError($errNo, $errMsg)) {
|
||||||
|
($this->log)('Hinweis: Antwort vom PayPal-Server wurde abgeschnitten (' . $errMsg . ').');
|
||||||
|
} else {
|
||||||
|
($this->log)('HTTP-Status: ' . $code . ($errMsg !== '' ? ' (' . $errMsg . ')' : ''));
|
||||||
|
}
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (self::isLengthMismatch($response, $declaredLen)) {
|
||||||
|
($this->log)(sprintf(
|
||||||
|
'Hinweis: Antwort scheint abgeschnitten zu sein (%d von %d Bytes empfangen).',
|
||||||
|
strlen($response),
|
||||||
|
(int)$declaredLen
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
|
if ($code !== 200 || $response === '') {
|
||||||
($this->log)('HTTP-Status: ' . $code);
|
($this->log)('HTTP-Status: ' . $code);
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
parse_str((string)$response, $result);
|
parse_str($response, $result);
|
||||||
return $result;
|
return $result;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** cURL meldet einen abgebrochenen/unvollständigen Transfer nicht immer über denselben Fehlercode. */
|
||||||
|
private static function isTruncatedTransferError(int $errNo, string $errMsg): bool
|
||||||
|
{
|
||||||
|
if (in_array($errNo, [CURLE_PARTIAL_FILE, CURLE_GOT_NOTHING, CURLE_RECV_ERROR], true)) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
return stripos($errMsg, 'transfer closed') !== false
|
||||||
|
|| stripos($errMsg, 'Empty reply') !== false;
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Erkennt eine Antwort, die trotz HTTP 200 kürzer ist als vom Server angekündigt. */
|
||||||
|
private static function isLengthMismatch(string $response, float $declaredLength): bool
|
||||||
|
{
|
||||||
|
return $declaredLength > 0 && strlen($response) < (int)$declaredLength;
|
||||||
|
}
|
||||||
|
|
||||||
private function testCredentials(): bool
|
private function testCredentials(): bool
|
||||||
{
|
{
|
||||||
$result = $this->nvpPost(['METHOD' => 'GetBalance']);
|
$result = $this->nvpPost(['METHOD' => 'GetBalance']);
|
||||||
|
|
|
||||||
96
tests/91_PayPalNvpAbgeschnitteneAntwortTest.php
Normal file
96
tests/91_PayPalNvpAbgeschnitteneAntwortTest.php
Normal file
|
|
@ -0,0 +1,96 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
declare(strict_types=1);
|
||||||
|
|
||||||
|
use Steuerkram\Services\PayPalNvpFetchService;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Test 91: PayPalNvpFetchService – Erkennung abgeschnittener Antworten (#185).
|
||||||
|
*
|
||||||
|
* Eine abgeschnittene NVP-Antwort darf nicht still verworfen werden, sondern
|
||||||
|
* muss als Hinweis im Abrufprotokoll landen. Deckt die beiden dafür
|
||||||
|
* zuständigen Hilfsmethoden isTruncatedTransferError() und isLengthMismatch()
|
||||||
|
* ab, die ohne echte PayPal-Verbindung testbar sind.
|
||||||
|
*/
|
||||||
|
class PayPalNvpAbgeschnitteneAntwortTest extends TestCase
|
||||||
|
{
|
||||||
|
protected string $testName = 'Test 91: PayPalNvpFetchService – abgeschnittene Antworten';
|
||||||
|
|
||||||
|
public function run(): void
|
||||||
|
{
|
||||||
|
$this->testIsTruncatedTransferErrorErkenntBekannteFehlercodes();
|
||||||
|
$this->testIsTruncatedTransferErrorErkenntFehlermeldungstext();
|
||||||
|
$this->testIsTruncatedTransferErrorIgnoriertNormaleFehler();
|
||||||
|
$this->testIsLengthMismatchErkenntKuerzereAntwort();
|
||||||
|
$this->testIsLengthMismatchOhneFehlalarm();
|
||||||
|
}
|
||||||
|
|
||||||
|
private function callPrivateStatic(string $method, array $args = []): mixed
|
||||||
|
{
|
||||||
|
$ref = new \ReflectionMethod(PayPalNvpFetchService::class, $method);
|
||||||
|
$ref->setAccessible(true);
|
||||||
|
return $ref->invokeArgs(null, $args);
|
||||||
|
}
|
||||||
|
|
||||||
|
private function testIsTruncatedTransferErrorErkenntBekannteFehlercodes(): void
|
||||||
|
{
|
||||||
|
foreach ([CURLE_PARTIAL_FILE, CURLE_GOT_NOTHING, CURLE_RECV_ERROR] as $errNo) {
|
||||||
|
$this->assert(
|
||||||
|
$this->callPrivateStatic('isTruncatedTransferError', [$errNo, '']) === true,
|
||||||
|
"isTruncatedTransferError() erkennt cURL-Fehlercode {$errNo} als abgeschnittenen Transfer"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private function testIsTruncatedTransferErrorErkenntFehlermeldungstext(): void
|
||||||
|
{
|
||||||
|
$this->assert(
|
||||||
|
$this->callPrivateStatic('isTruncatedTransferError', [
|
||||||
|
0,
|
||||||
|
'transfer closed with 128 bytes remaining to read',
|
||||||
|
]) === true,
|
||||||
|
'isTruncatedTransferError() erkennt "transfer closed"-Meldungen auch bei unbekanntem Fehlercode'
|
||||||
|
);
|
||||||
|
$this->assert(
|
||||||
|
$this->callPrivateStatic('isTruncatedTransferError', [0, 'Empty reply from server']) === true,
|
||||||
|
'isTruncatedTransferError() erkennt "Empty reply from server"'
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
private function testIsTruncatedTransferErrorIgnoriertNormaleFehler(): void
|
||||||
|
{
|
||||||
|
$this->assert(
|
||||||
|
$this->callPrivateStatic('isTruncatedTransferError', [0, '']) === false,
|
||||||
|
'isTruncatedTransferError() meldet ohne Fehlercode/-text keinen Abbruch'
|
||||||
|
);
|
||||||
|
$this->assert(
|
||||||
|
$this->callPrivateStatic('isTruncatedTransferError', [CURLE_COULDNT_CONNECT, 'Connection refused']) === false,
|
||||||
|
'isTruncatedTransferError() stuft einen Verbindungsfehler nicht als abgeschnittene Antwort ein'
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
private function testIsLengthMismatchErkenntKuerzereAntwort(): void
|
||||||
|
{
|
||||||
|
$this->assert(
|
||||||
|
$this->callPrivateStatic('isLengthMismatch', ['ACK=Success', 100.0]) === true,
|
||||||
|
'isLengthMismatch() erkennt eine Antwort, die kürzer als Content-Length ist'
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
private function testIsLengthMismatchOhneFehlalarm(): void
|
||||||
|
{
|
||||||
|
$vollstaendig = 'ACK=Success';
|
||||||
|
$this->assert(
|
||||||
|
$this->callPrivateStatic('isLengthMismatch', [$vollstaendig, (float)strlen($vollstaendig)]) === false,
|
||||||
|
'isLengthMismatch() schlägt bei übereinstimmender Länge nicht an'
|
||||||
|
);
|
||||||
|
$this->assert(
|
||||||
|
$this->callPrivateStatic('isLengthMismatch', ['ACK=Success', 0.0]) === false,
|
||||||
|
'isLengthMismatch() schlägt ohne bekannte Content-Length (0/-1) nicht an'
|
||||||
|
);
|
||||||
|
$this->assert(
|
||||||
|
$this->callPrivateStatic('isLengthMismatch', ['ACK=Success', -1.0]) === false,
|
||||||
|
'isLengthMismatch() schlägt bei negativer Content-Length nicht an'
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Add table
Reference in a new issue