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

エスケープ漏れその他 #3

Closed
6 tasks done
miya0001 opened this issue Aug 30, 2018 · 2 comments
Closed
6 tasks done

エスケープ漏れその他 #3

miya0001 opened this issue Aug 30, 2018 · 2 comments
Labels
good first issue Good for newcomers
Milestone

Comments

@miya0001
Copy link

miya0001 commented Aug 30, 2018

たまたま目についたのでつっこみを。笑

  • 以下 esc_url() でエスケープするべき

https://github.com/mayukojpn/force-update-translations/blob/master/force-update-translations.php#L34

  • バリデーション漏れ。公式ディレクトリ上のWordPressプラグインのスラッグには半角英数とハイフォンしか使えない(?)と思われるので、正規表現等でそのようにチェックするべき。これはかなりやばい。笑

https://github.com/mayukojpn/force-update-translations/blob/master/force-update-translations.php#L52

  • エスケープ漏れ。esc_attr()esc_html()でエスケープするべき。

https://github.com/mayukojpn/force-update-translations/blob/master/force-update-translations.php#L138-L139

  • 同上

https://github.com/mayukojpn/force-update-translations/blob/master/force-update-translations.php#L71-L72

  • wp-に続く文字は半角英数以外(?)を想定する必要が無いのでは?この正規表現ではスラッシュも含めたすべての文字を通してしまうので、その後の file_put_contents() で任意のファイルをぶっ壊すことができる可能性があります。(実際にできるかどうかではなく、そういう心配をしなくてもいいようにするべき。)

https://github.com/mayukojpn/force-update-translations/blob/master/force-update-translations.php#L89

https://github.com/mayukojpn/force-update-translations/blob/master/force-update-translations.php#L105

  • esc_url_raw() でエスケープするべき。(esc_url() じゃないよ)

https://github.com/mayukojpn/force-update-translations/blob/master/force-update-translations.php#L125

エスケープは実際にそれが必要かどうかに関係なく使うシチュエーションにあわせてとりあえずやると決めるのが良いです。たとえばHTMLの属性に使うなら esc_attr() 、リンクに使用する URL なら esc_url() などのように利用目的に応じて問答無用でエスケープするのがよいです。

あとファイルに書き込むとかファイルを読み込むとかする場合は、特に慎重にやったほうがいいです。
https://ja.wikipedia.org/wiki/%E3%83%87%E3%82%A3%E3%83%AC%E3%82%AF%E3%83%88%E3%83%AA%E3%83%88%E3%83%A9%E3%83%90%E3%83%BC%E3%82%B5%E3%83%AB

@mayukojpn
Copy link
Owner

@miya0001 わかりやすくありがとうございます!直してみます!

mayukojpn added a commit that referenced this issue Aug 31, 2018
@mayukojpn mayukojpn added this to the 0.2.2 milestone Aug 31, 2018
mayukojpn added a commit that referenced this issue Aug 31, 2018
mayukojpn added a commit that referenced this issue Aug 31, 2018
mayukojpn added a commit that referenced this issue Sep 3, 2018
@mayukojpn mayukojpn modified the milestones: 0.2.2, 0.2.3 Sep 10, 2018
@mayukojpn mayukojpn added the good first issue Good for newcomers label Sep 10, 2018
@mayukojpn mayukojpn modified the milestones: 0.2.3, 0.2.4 Sep 26, 2018
@mayukojpn
Copy link
Owner

Escaping is my pain point that I haven't studied enough. 😭 Thanks for helping!

mayukojpn added a commit that referenced this issue Sep 15, 2020
mayukojpn added a commit that referenced this issue Sep 15, 2020
Add contributor. related: #3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants