0
✦ 品質
🛡 セキュリティ
⚙ 保守性
📋 概要
C++17とOpenGL 3.3 Coreで実装されたデータドリブンな2×4木造軸組フレームのミニCADツール。外部の数学・JSON・UIライブラリを一切使用せず、行列演算・レイピッキング・JSONパーサ・ストロークフォントをすべて自前実装している。
🎯 今すぐ直すべき3点
-
src/json_parser.cpp(推定) 自作JSONパーサのバッファ境界チェックを全入力パスで検証し、malformedなhouse.jsonを与えた際のバッファオーバーリードやUB(未定義動作)を防ぐためにfuzz testingを導入する↗
-
house.json 入力値(幅・奥行・窓位置など)のバリデーションロジックをアプリ起動時に一元化し、不正値(負数・ゼロ・極端に大きい値)を受け取ったときに安全にフォールバックまたはエラー終了する処理を追加する↗
-
CMakeLists.txt -Wall -Wextra -fsanitize=address,undefined をCMakeのDebugビルド設定に追加し、自作行列演算・JSONパーサ・レイピッキングコード内の潜在的なUBとメモリエラーを静的・動的解析で炙り出す↗
🔍 検出された問題 (6件)
🟥 Critical
📄 src/json_parser.cpp(推定)
自作JSONパーサへの未検証入力によるバッファオーバーリードの危険
house.jsonを外部入力として自作パーサで読み込んでいるが、標準ライブラリのみで実装された自作パーサは境界チェックの漏れが起きやすく、malformedなJSON(未終端文字列・深いネスト・極端に長いキー)を与えると未定義動作またはクラッシュが発生する可能性がある。悪意ある設定ファイルを配布された場合に任意コード実行につながるリスクがある。
🟥 Critical
📄 src/math.cpp(推定)
自作行列演算コードにおける未定義動作(UB)の潜在リスク
glmを使わずに自前実装したmat4演算・クォータニオン・レイピッキングのコードは、符号付き整数オーバーフローやゼロ除算(レイと平行な面との交差判定など)によりUBが発生する可能性がある。AddressSanitizerやUBSanitizerによる検証が実施されていない場合、リリースビルドで最適化により誤動作が無音で発生する。
🟧 Warning
📄 src/csv_export.cpp(推定)
CSVエクスポート関数でのパス・ファイル名の未サニタイズ
READMEにCSVエクスポート機能(Cキー)が記載されているが、出力ファイルパスの生成でユーザー入力やhouse.jsonの値をそのまま使用している場合、パストラバーサル(../../../etc/passwd相当)によって意図しないパスにファイルが書き込まれるリスクがある。
🟧 Warning
📄 src/stroke_font.cpp(推定)
自作ストロークフォントの文字列描画で任意長文字列を想定していない可能性
部材IDや寸法数値を世界座標にストロークフォントで描画する実装において、文字列バッファの長さ上限が固定されている場合、長いIDや大きな数値が切り捨てられるか、バッファオーバーランが発生する可能性がある。
🟨 Suggestion
テストコードが見当たらず、手続き的フレーム生成ロジックの正確性を検証できない
2×4フレームを手続き的に生成する核心ロジック(スタッド間隔・ヘッダー寸法・窓/ドア開口部の生成)に対してユニットテストが存在しないと、house.jsonの仕様変更時に部材数や位置の回帰バグを検出できない。
🟨 Suggestion
📄 README.md
README.md内の説明文が途中で切れており、CSVエクスポートの仕様が不完全
README.mdの「CSV エクスポート — `C` キ」で文章が途中で終わっており、ユーザーが機能の完全な仕様を把握できない。ドキュメントの不完全さは保守者の引き継ぎコストを増加させる。
✅ 良い点
- 外部数学ライブラリ(glmなど)を使わずにmat4/vec3演算・カメラ・レイピッキングをすべて自前実装しており、コードの依存関係が最小限に抑えられているため、ビルド環境の再現性が高い
- house.jsonによるデータドリブン設計が採用されており、建物仕様の変更がコード変更なしに行えるため、ドメインロジック(フレーム生成)とUIの分離が明確になっている
- 全部材にID・ゾーン・断面・材長・材種の属性が付与されており(例: S-014, South Wall, 38x89mm)、BIMの入口となる部材識別の設計が将来の拡張(IFC連携等)に対応しやすい構造になっている
🔧 リファクタ提案
- 自作JSONパーサをnlohmann/jsonまたはrapidJSON(ヘッダーオンリー)に差し替えることでパーサ起因のバグリスクをゼロにする。READMEの『外部ライブラリ不使用』という訴求点はmath/rendering自作で十分に維持できる
- src/math.hppの自作vec3/mat4演算クラスにoperator overloadとconstexprを徹底適用し、コンパイル時計算可能な部分をconstexpr化することでランタイムUBの発生箇所を減らす
- フレーム生成ロジック(FrameGenerator相当クラス)をOpenGLレンダリングコードから完全分離し、生成結果をstd::vector<Member>として返すピュアな関数にすることで、ヘッドレスでのユニットテストとCSVエクスポートの両方から再利用できるようにする
🔒 セキュリティ注意点
- house.jsonは外部から差し替え可能な設定ファイルであるため、読み込み時に全フィールドの型・範囲・必須チェックを行うバリデーション関数をmain()またはFrameConfig::load()の直後に必ず挿入すること
- CSVエクスポートのファイルパス生成には std::filesystem::canonical() を使用してパストラバーサルを防ぎ、書き込み先ディレクトリをアプリケーション実行ディレクトリ以下に制限すること
📤 PRコメント出力
このレビューをGitHub PRコメントとして貼り付けられるMarkdown形式で出力します。