Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SonarScanで検出されたvulnerabilitiesを解消する #1552

Merged

Conversation

berryzplus
Copy link
Contributor

PR の目的

SonarScanで検出されたvulnerabilitiesを解消します。

カテゴリ

  • リファクタリング

PR の背景

SonarScanでvulnerabilitiesが検出されています。

vulnerability は日本語で言うとセキュリティ脆弱性にあたります。

放置しておくのも微妙なので、何かしら対処したいと思っています。

内容は、sprintf関数にはオーバーフローを起こすリスクがあるので問題がないか確認せよ、というものです。

結論的には、指摘された2箇所のsprintfは出力文字数を指定しているので、バッファオーバーフロー問題が起きる可能性はゼロです。一応、この脆弱性に「問題ないっす!」とコメントしてやれば「なかったこと」にできます。

ただまぁ、脆弱性が検出されるコードを残しておく意味もないので、対応してしまいたいと思います。

PR のメリット

  • SonarScanのvulnerabilitiesが解消します。
  • CClipboardクラスの一部にテストが追加されます。

PR のデメリット (トレードオフとかあれば)

  • とくにないように思います。
    修正が必要か?と考えた場合に「必ずしもそうではない」という点がデメリットになります。

仕様・動作説明

仕様・挙動を変えない修正なので割愛します。

PR の影響範囲

CClipboard::SetHtmlTextの結果のみに影響する修正です。

テスト内容

変更を加える前のコードで単体テストを作成して実行し、
変更を加えた後のコードで結果が変わらないことを確認しました。

関連 issue, PR

参考資料

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@berryzplus berryzplus marked this pull request as ready for review February 23, 2021 14:02
@AppVeyorBot
Copy link

@berryzplus
Copy link
Contributor Author

レビューありがとうございます。マージしちゃいます。

@berryzplus berryzplus merged commit ec41bc5 into sakura-editor:master Feb 23, 2021
@berryzplus berryzplus deleted the feature/fix_vulnerabilities branch February 23, 2021 23:23
@berryzplus
Copy link
Contributor Author

マージ後ビルドでvulnerabilitiesを解消したのを確認しました。

@beru beru added the refactoring リファクタリング 【ChangeLog除外】 label Mar 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring リファクタリング 【ChangeLog除外】
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants