diff --git a/Changes.md b/Changes.md index 736e71e..036cd23 100644 --- a/Changes.md +++ b/Changes.md @@ -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) - `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. diff --git a/src/Services/PayPalNvpFetchService.php b/src/Services/PayPalNvpFetchService.php index 86688b9..9ab8301 100644 --- a/src/Services/PayPalNvpFetchService.php +++ b/src/Services/PayPalNvpFetchService.php @@ -276,19 +276,55 @@ class PayPalNvpFetchService CURLOPT_HTTPHEADER => ['Content-Type: application/x-www-form-urlencoded'], CURLOPT_TIMEOUT => 30, ]); - $response = curl_exec($ch); - $code = curl_getinfo($ch, CURLINFO_HTTP_CODE); + $response = curl_exec($ch); + $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); - 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); return null; } - parse_str((string)$response, $result); + parse_str($response, $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 { $result = $this->nvpPost(['METHOD' => 'GetBalance']); diff --git a/tests/91_PayPalNvpAbgeschnitteneAntwortTest.php b/tests/91_PayPalNvpAbgeschnitteneAntwortTest.php new file mode 100644 index 0000000..cd48343 --- /dev/null +++ b/tests/91_PayPalNvpAbgeschnitteneAntwortTest.php @@ -0,0 +1,96 @@ +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' + ); + } +}