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

pixiv-winter-internship 脆弱性対処 #6

Closed

Conversation

takechanman1228
Copy link

私が対処したこと

  1. SQL injection対策
  2. HTMLやMarkdownを許しつつXSS対策
  3. ユーザーのパスワードをハッシュ化
  4. [UI]ユーザーがログイン失敗した時の簡単なエラーの表示
  5. GETでログイン時していた部分をPOSTでログイン
  6. パスワード入力欄にはtype="password"を指定

説明

1. SQL injection対策

  • 脆弱性箇所:チャットでのメッセージ投稿時など
  • 原因:外部から入力された文字列がエスケープされずSQLが実行されている。
  • 対処方法:PDO::bindvalueを利用してプレースホルダの組み立てた。
  • 修正コード: fcb19bb

2. HTMLやMarkdownを許しつつXSS対策

  • 脆弱性箇所:チャットでのメッセージでは<script>alert(document.cookie);</script>などが実行でき、XSS攻撃が可能
  • 原因:チャットでのメッセージは、HTMLやmarkdownで投稿、表示できるよう(意図的に)タグ等がエスケープされていない。そのため、悪意あるスクリプトが実行可能となっている。私の確認ではその他の箇所はタグが適切にエスケープされていた。
  • 対処方法:HTML purifierを利用。入力されたチャットでのMarkdownメッセージをParsedownでパースした後、HTML purifierでXSSの可能性がある危険なコードを除去した。
    この対処により、HTMLやMarkdownでのメッセージ投稿を可能につつ、悪意あるスクリプトによるXSS攻撃を防ぐことができる。
  • 修正コード: 01b15ff (別途composer updateが必要)
    screen shot 2015-11-30 at 19 36 12

3. ユーザーのパスワードをハッシュ化

  • 脆弱性箇所:DBにパスワードが平文で保存されているため、万が一DBの内容が漏洩した場合に被害が非常に大きい。
  • 原因:ユーザー登録時にパスワードを平文でハッシュ化処理をしていない。
  • 対処方法:password_hash関数を利用してパスワードをハッシュ化し、password_verify関数を利用してログイン時にパスワードチェックを行うようにした。
  • 修正コード: 2a865e3

4. [UI]ユーザーがログイン失敗した時の簡単なエラーの表示

  • 不親切な箇所:ログインが失敗しても何も表示されず、ユーザーが何が起こったか理解できない。
  • 対処方法:ログインに失敗した際はその旨を簡単に表示
  • 修正コード: 2a865e3

screen shot 2015-11-30 at 19 47 53

### 5. GETでログイン時していた部分をPOSTでログイン - 脆弱性箇所:ユーザーログイン時 - 原因:ユーザーログイン処理をGETで行っているため、 パスワードを含むURLアクセスすることになる。 - 対処方法:POSTで処理を行う - 修正コード: 4c8f002

screen shot 2015-11-30 at 19 58 56

### 6. パスワード入力欄にはtype="password"を指定 - 脆弱性箇所:ユーザーログインのパスワード入力欄でパスワードが丸見え - 対処方法:type="password"を指定 - 修正コード: e14f19f

@zonuexe
Copy link
Contributor

zonuexe commented Nov 30, 2015

👍 受理しました

@zonuexe zonuexe closed this Nov 30, 2015
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.

None yet

2 participants