Skip to content

fix: Web SAPI の fatal error と read() のメモリ枯渇などを直す(v1.3.4) - #18

Merged
tomio2480 merged 9 commits into
mainfrom
fix/harden-runtime-safety
Oct 21, 2026
Merged

tomio2480 merged 9 commits into
mainfrom
fix/harden-runtime-safety

Conversation

@tomio2480

Copy link
Copy Markdown
Owner

セルフレビュー実施済み(self-reviewer による一次走査と /security-review を実行).

概要

2026-10-21 の PHP セキュリティアドバイザリ(CVE-2026-109618 / CVE-2026-109619)を受けて,ライブラリを再点検した.
PHP 本体の脆弱性はライブラリ側では直せない.一方,アドバイザリが挙げる緩和策に関わる弱点が見つかったため,5 件を直す.
v1.3.4 として出す想定である.

対応する Issue はない.

変更内容

  1. Web 系 SAPI での fatal error を防ぐ(Windows.php)
    FFI 無効の Windows では警告を fwrite(STDERR, ...) で出していた.
    STDERR は CLI でのみ定義されるため,php -S・Apache・FPM では Windows クラスの生成時点で fatal error になっていた.
    STDERR が無ければ error_log() へ送る.アドバイザリの「攻撃者が到達できる fatal error を直す」に当たる.
  2. read() の長さを 64 KiB へ丸める(新設 ReadLength,各プラットフォーム)
    fread() と FFI::new("char[n]") は指定長を先に確保する.上限がなく,巨大な指定で memory_limit を超えて fatal error になっていた.
    CVE-2026-109618 の引き金と同種のメモリ不足である.read() は最大 n バイトを返す API のため,例外にせず上限へ丸める.
  3. デバイス名の末尾改行を拒否する(全プラットフォーム)
    PCRE の $ は末尾改行の手前でも一致し,"COM1\n" が通っていた.D 修飾子を付ける.
  4. WindowsFFI::write() の送信バッファを解放する
    FFI::new(..., false) の確保を解放しておらず,送信のたびにメモリが増えていた.所有権付きの確保に変える.
  5. WindowsFFI::open() の失敗を判定する
    FFI\CData 同士の === はオブジェクトの同一性を比べ,常に偽だった.ポインタ値を比べる == に変える.

README と docs/WINDOWS_FFI_SETUP.md に,read() の上限と警告の出力先を追記した.
ReadLength は内部部品として @internal を付け,公開 API を増やさない.

セルフレビューで見送った指摘

  • Windows.php のパリティ検証 /^[noe]$/ に D 修飾子がない.値は固定の対応表からしか来ず,改行は入りえないため見送った.
  • 閉じられた STDERR への fwrite が TypeError になりうる.利用者が自ら STDERR を閉じる稀な場合に限られ,今回の範囲外とした.

互換性

  • 公開 API のシグネチャは変えていない.
  • read() に 64 KiB を超える長さを渡すと,返る最大長が 64 KiB になる.もともと「最大 n バイト」の API のため,patch リリースの範囲とした.
  • 末尾に改行を含むデバイス名は RuntimeException になる.従来も正しく動作しない入力である.

テスト

各修正は Red を確認してから実装した(TDD).

テスト 内容
tests/WebSapiTest.php php -n -S の実プロセスで fixture を実行し,fatal にならず警告がログへ出ることを確かめる
tests/ReadLengthTest.php 境界値(0・1・上限・上限+1・PHP_INT_MAX・PHP_INT_MIN)と,Unix / Windows の丸め
tests/SecurityTest.php 末尾改行付きのデバイス名を全プラットフォームで拒否する
tests/WindowsFFITest.php 存在しない COM255 の open 失敗,無効ハンドルへの write 反復でメモリが増えないこと(Windows + FFI のみ)

ローカル(Windows,PHP 8.5.12)で composer test・cs:check・cs:fixer・stan が通過した.
PHPUnit Deprecations: 1 は phpunit.xml のスキーマ警告で,本 PR 以前から出ている.

ドキュメント整合性

README・docs/WINDOWS_FFI_SETUP.md を差分と照合し,必要な追記を本 PR に含めた..env.example・CONTRIBUTING.md は変更不要と確認した.

リリースノート案(v1.3.4)

## セキュリティ・安定性の修正

- Web サーバー経由(Apache・PHP-FPM・`php -S` など)で FFI 無効の Windows 実装を使うと,STDERR 未定義による fatal error になる問題を修正
- `read()` の読み取り長を 64 KiB へ丸め,巨大な指定によるメモリ枯渇を防止
- デバイス名の検証で,末尾に改行を含む名前を拒否
- `WindowsFFI::write()` の送信バッファが解放されない問題を修正
- `WindowsFFI::open()` がポートを開けなかったことを検出できない問題を修正

PHP 本体の CVE-2026-109618 / CVE-2026-109619 は本ライブラリでは修正できません.
PHP 8.5.12・8.4.27・8.3.36・8.2.35 以降への更新を推奨します.

マージ後,v1.3.4 タグの作成と GitHub Release の公開を行う(Packagist はタグで更新される).

🤖 Generated with Claude Code

tomio2480 and others added 8 commits October 21, 2026 23:50
FFI 無効の Windows では,警告を fwrite(STDERR, ...) で出していた.
STDERR は CLI でのみ定義されるため,php -S・Apache・FPM などでは
Windows クラスの生成時点で Error が投げられ,fatal error になっていた.

STDERR があれば従来どおり書き出し,無ければ error_log() へ送る.
内蔵 Web サーバーの実プロセスで fixture を実行するテストを加えた.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fread() と FFI::new("char[n]") は指定長のバッファを先に確保する.
長さに上限がなく,外部入力をそのまま渡すと memory_limit を超えて
fatal error になった.CVE-2026-109618 はこの種のメモリ不足処理が
引き金であり,未修正の PHP では攻撃の入口にもなりうる.

検証と丸めを ReadLength へ集め,Unix・Windows・WindowsFFI から使う.
read() は最大 n バイトを返す API のため,例外にせず上限へ丸める.
1 未満は従来どおり例外とし,WindowsFFI にも同じ検証を入れた.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PCRE の $ は末尾の改行の手前でも一致するため,"COM1\n" や
"/dev/ttyUSB0\n" が検証を通っていた.Windows 版はこの名前を
cmd.exe の mode コマンドへそのまま渡す.後ろへ命令を足せないため
注入には至らないが,検証の意図と食い違う.

全プラットフォームの正規表現へ D 修飾子を付け,文字列の末尾でのみ
一致させる.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CreateFileA の戻り値と (HANDLE)-1 を === で比べていた.FFI\CData
同士の === はオブジェクトの同一性を比べるため常に偽となり,失敗を
見逃して無効なハンドルを返していた.

== はポインタ値を比べるため,こちらで INVALID_HANDLE_VALUE を判定する.
存在しない COM255 を開くテストを Windows + FFI 環境向けに加えた.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
送信バッファを FFI::new(..., false) で確保していた.所有権を持たない
確保は FFI::free() を呼ぶまで解放されず,送信のたびにメモリが増えた.
常駐プロセスでは,やがて memory_limit に達する.

WriteFile は同期呼び出しのため,所有権付きで確保して破棄と同時に
解放させる.無効なハンドルへの書き込みを繰り返し,メモリ増分を
測るテストを加えた(修正前は 8 回で約 8 MiB 増えていた).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
戻り値は常に 1 以上のため,PHPDoc で int<1, max> と宣言する.
PHPStan が fread() の length 引数(int<1, max>)との不一致を報告していた.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
read() が 64 KiB へ丸めることと,1 未満で例外になることを書く.
FFI 無効時の警告は,Web 系 SAPI ではエラーログへ出ることを書く.
あわせて,セキュリティ修正が提供される PHP の利用を推奨する.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ReadLength は Platform 実装の内部部品のため @internal を付け,README
からクラス名を外す.patch リリースで公開 API を増やさないためである.

WebSapiTest は,CI ランナーの起動の遅さを見込んで待ち時間を 15 秒へ
延ばす.子プロセスが先に終了した場合はログを添えて即座に落とし,
一時ファイルは finally で必ず消す.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Lint summary

ツール この PR の差分 リポジトリ全体
markdownlint 45 58
textlint 84 (error: 84) 226

検査対象: Merge 2e83ad9 into bda92d5(base: bda92d5)

差分行に該当する指摘は inline コメントとして該当行に付きます.filter-mode(既定 added)の都合で inline 化されない指摘は下の details 一覧から確認してください.リポジトリ全体の内訳は Actions run の artifact lint-reports-lint にある markdownlint-report.txt と textlint-report.xml から確認できます.

markdownlint の指摘 45 件
  • CLAUDE.md:20 MD040/fenced-code-language: Fenced code blocks should have a language specified [Context: "```"]
  • CLAUDE.md:38 MD022/blanks-around-headings: Headings should be surrounded by blank lines [Expected: 1; Actual: 0; Below] [Context: "### 実装方針"]
  • CLAUDE.md:39 MD032/blanks-around-lists: Lists should be surrounded by blank lines [Context: "1. Pure PHP実装: PHP標準機能とFFI..."]
  • CLAUDE.md:43 MD022/blanks-around-headings: Headings should be surrounded by blank lines [Expected: 1; Actual: 0; Below] [Context: "### クラス設計"]
  • CLAUDE.md:44 MD032/blanks-around-lists: Lists should be surrounded by blank lines [Context: "- SerialPort: ユーザー向けAPI。open..."]
  • CLAUDE.md:52 MD022/blanks-around-headings: Headings should be surrounded by blank lines [Expected: 1; Actual: 0; Below] [Context: "### 依存関係のインストール"]
  • CLAUDE.md:53 MD031/blanks-around-fences: Fenced code blocks should be surrounded by blank lines [Context: "```bash"]
  • CLAUDE.md:57 MD022/blanks-around-headings: Headings should be surrounded by blank lines [Expected: 1; Actual: 0; Below] [Context: "### テスト実行"]
  • CLAUDE.md:58 MD031/blanks-around-fences: Fenced code blocks should be surrounded by blank lines [Context: "```bash"]
  • CLAUDE.md:69 MD022/blanks-around-headings: Headings should be surrounded by blank lines [Expected: 1; Actual: 0; Below] [Context: "### コーディングスタイルチェック"]
  • CLAUDE.md:70 MD031/blanks-around-fences: Fenced code blocks should be surrounded by blank lines [Context: "```bash"]
  • CLAUDE.md:77 MD022/blanks-around-headings: Headings should be surrounded by blank lines [Expected: 1; Actual: 0; Below] [Context: "### ユニットテスト"]
  • CLAUDE.md:78 MD032/blanks-around-lists: Lists should be surrounded by blank lines [Context: "- 設定クラスのバリデーション"]
  • CLAUDE.md:82 MD022/blanks-around-headings: Headings should be surrounded by blank lines [Expected: 1; Actual: 0; Below] [Context: "### モックテスト"]
  • CLAUDE.md:83 MD032/blanks-around-lists: Lists should be surrounded by blank lines [Context: "- 実際のシリアルポートなしでテスト可能にする"]
  • CLAUDE.md:86 MD022/blanks-around-headings: Headings should be surrounded by blank lines [Expected: 1; Actual: 0; Below] [Context: "### 統合テスト (任意)"]
  • CLAUDE.md:87 MD032/blanks-around-lists: Lists should be surrounded by blank lines [Context: "- 実際のシリアルデバイスを使用したテスト"]
  • CLAUDE.md:92 MD022/blanks-around-headings: Headings should be surrounded by blank lines [Expected: 1; Actual: 0; Below] [Context: "### 環境"]
  • CLAUDE.md:93 MD032/blanks-around-lists: Lists should be surrounded by blank lines [Context: "- PHP 8.5とComposerがインストール済み"]
  • CLAUDE.md:96 MD022/blanks-around-headings: Headings should be surrounded by blank lines [Expected: 1; Actual: 0; Below] [Context: "### ファイルハンドル管理"]
  • ...他 25 件
textlint の指摘 84 件
  • README.md:6 [error] ja-spacing/ja-space-between-half-and-full-width: 原則として、全角文字と半角文字の間にスペースを入れます。 (ja-spacing/ja-space-between-half-and-full-width)
  • README.md:6 [error] ja-technical-writing/ja-no-redundant-expression: 【dict5】 "通信を行う"は冗長な表現です。"通信する"など簡潔な表現にすると文章が明瞭になります。 解説: https://github.com/textlint-ja/textlint-rule-ja-no-redundant-expression#dict5 (ja-technical-writing/ja-no-redundant-expression)
  • README.md:10 [error] ja-spacing/ja-space-between-half-and-full-width: 原則として、全角文字と半角文字の間にスペースを入れます。 (ja-spacing/ja-space-between-half-and-full-width)
  • README.md:10 [error] ja-spacing/ja-space-between-half-and-full-width: 原則として、全角文字と半角文字の間にスペースを入れます。 (ja-spacing/ja-space-between-half-and-full-width)
  • README.md:10 [error] ja-spacing/ja-space-between-half-and-full-width: 原則として、全角文字と半角文字の間にスペースを入れます。 (ja-spacing/ja-space-between-half-and-full-width)
  • README.md:11 [error] ja-spacing/ja-space-between-half-and-full-width: 原則として、全角文字と半角文字の間にスペースを入れます。 (ja-spacing/ja-space-between-half-and-full-width)
  • README.md:11 [error] ja-spacing/ja-space-between-half-and-full-width: 原則として、全角文字と半角文字の間にスペースを入れます。 (ja-spacing/ja-space-between-half-and-full-width)
  • README.md:15 [error] ja-spacing/ja-space-between-half-and-full-width: 原則として、全角文字と半角文字の間にスペースを入れます。 (ja-spacing/ja-space-between-half-and-full-width)
  • README.md:22 [error] ja-spacing/ja-space-between-half-and-full-width: 原則として、全角文字と半角文字の間にスペースを入れます。 (ja-spacing/ja-space-between-half-and-full-width)
  • README.md:72 [error] ja-technical-writing/max-comma: This sentence exceeds the maximum count of comma. Maximum is 3. (ja-technical-writing/max-comma)
  • README.md:72 [error] ja-technical-writing/sentence-length: Line 72 sentence length(85) exceeds the maximum sentence length of 80. Over 5 characters. (ja-technical-writing/sentence-length)
  • README.md:131 [error] ja-spacing/ja-space-around-code: インラインコードの後にスペースを入れてください。 (ja-spacing/ja-space-around-code)
  • README.md:131 [error] ja-spacing/ja-space-between-half-and-full-width: 原則として、全角文字と半角文字の間にスペースを入れます。 (ja-spacing/ja-space-between-half-and-full-width)
  • README.md:131 [error] ja-spacing/ja-space-between-half-and-full-width: 原則として、全角文字と半角文字の間にスペースを入れます。 (ja-spacing/ja-space-between-half-and-full-width)
  • README.md:137 [error] ja-spacing/ja-space-between-half-and-full-width: 原則として、全角文字と半角文字の間にスペースを入れます。 (ja-spacing/ja-space-between-half-and-full-width)
  • README.md:138 [error] ja-spacing/ja-space-around-link: リンクの前にスペースを入れてください。 (ja-spacing/ja-space-around-link)
  • README.md:138 [error] ja-spacing/ja-space-around-link: リンクの後にスペースを入れてください。 (ja-spacing/ja-space-around-link)
  • README.md:169 [error] ja-spacing/ja-space-around-link: リンクの前にスペースを入れてください。 (ja-spacing/ja-space-around-link)
  • README.md:169 [error] ja-spacing/ja-space-around-link: リンクの後にスペースを入れてください。 (ja-spacing/ja-space-around-link)
  • README.md:170 [error] ja-spacing/ja-space-around-link: リンクの前にスペースを入れてください。 (ja-spacing/ja-space-around-link)
  • ...他 64 件

Actions run: https://github.com/tomio2480/php-serial/actions/runs/39623855448/attempts/1

@coderabbitai

coderabbitai Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: cdfd76b0-ccee-4539-9197-a6967aa2e705

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

全角文字と半角文字の間へ半角スペースを入れる.

README.md と docs/WINDOWS_FFI_SETUP.md は利用者向けの手引きで,
本文全体が「ですます調」・句点「。」で書かれている.追記行だけを
である調や「.」へ変えると 1 ファイル内で混ざるため,文体と句点の
2 ルールをファイル単位のコメントで無効化する.判断は CLAUDE.md に記す.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tomio2480

Copy link
Copy Markdown
Owner Author

@codex review

@tomio2480

Copy link
Copy Markdown
Owner Author

reviewdog レビュー対応

2e83ad9 で対応した.再実行後,追記行へのインライン指摘は 0 件である.

  • README.md:15: 全角と半角の間のスペース → 採用.「PHP 本体」へ修正済み.
  • README.md:15: である調の要求 → 却下(仕様上の意図).README は利用者向けの手引きで,全体が「ですます調」である.ファイル先頭のコメントで当該ルールを無効化した.
  • README.md:15: 句点「.」の要求 → 却下(仕様上の意図).既存の本文が「。」で統一されており,追記行だけ変えると 1 ファイル内で混ざる.ファイル先頭のコメントで当該ルールを無効化した.
  • README.md:61: である調の要求 → 却下.理由は README.md:15 と同じ.
  • README.md:62(3 件): 全角と半角の間のスペース → 採用.「1 回」「64 KiB で」へ修正済み.
  • README.md:62: である調の要求 → 却下.理由は README.md:15 と同じ.
  • README.md:63: 全角と半角の間のスペース → 採用.「1 未満」へ修正済み.
  • README.md:63: である調の要求 → 却下.理由は README.md:15 と同じ.
  • README.md:63: 句点「.」の要求 → 却下.理由は README.md:15 と同じ.
  • docs/WINDOWS_FFI_SETUP.md:14・:15: である調の要求 → 却下.利用者向けの手順書で,全体が「ですます調」である.ファイル先頭のコメントで無効化した.
  • docs/WINDOWS_FFI_SETUP.md:15: 句点「.」の要求 → 却下.既存の本文が「。」で統一されている.ファイル先頭のコメントで無効化した.

文体の判断はリポジトリの CLAUDE.md に 1 行で記した.
lint summary の「この PR の差分」欄の件数は,変更したファイルに元からある指摘を含む.既存行の指摘はスコープ外として,本 PR では扱わない.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-21T15:30:42.889080Z 2e83ad9 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Swish!

Reviewed commit: 2e83ad92ce

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@tomio2480
tomio2480 marked this pull request as ready for review October 21, 2026 15:49
@tomio2480
tomio2480 merged commit d52b49f into main Oct 21, 2026
21 checks passed
@tomio2480
tomio2480 deleted the fix/harden-runtime-safety branch October 21, 2026 16:07
tomio2480 added a commit that referenced this pull request Oct 22, 2026
#18 で下した判断(STDERR のフォールバック,read() の丸め,Web SAPI の
再現方法)と,棄却した代替案を記録する.FFI の CData 比較・所有権の
想定外の挙動と,stacked PR の #19 が自動クローズされた経緯も残す.

あわせて CLAUDE.md の文体方針に,docs/notes/ は判断の記録として
「である調」で書くことを加える.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant