『CodeIgniter徹底入門』のサンプルコードにXSS脆弱性が見つかった話

『CodeIgniter徹底入門』に掲載されている「簡易ショッピングサイト」のコードにXSS脆弱性が発見されました。発見したのは私です。なので、それでちゃらにしてください。

XSS脆弱性の内容

エスケープ漏れによる反射型のXSSです。

該当する箇所は以下になります。

<p class="coment"><?=$total_item?></p>

https://github.com/kenjis/codeigniter-tettei-apps/blob/62b2a4b05fcad2ab4556b33acd6beca0dc142cfe/system/application/views/shop_search.php#L5

修正パッチ

単純なエスケープ漏れなので、修正はエスケープすればOKです。

--- a/7-9/system/application/views/shop_search.php
+++ b/7-9/system/application/views/shop_search.php
@@ -2,7 +2,7 @@

 <?=$pagination?>

-<p class="coment"><?=$total_item?></p>
+<p class="coment"><?=form_prep($total_item);?></p>

 <?php foreach($list as $row): ?>
 <a href="<?=base_url();?>shop/product/<?=$row->id?>">

form_prep()関数は古いCodeIgniterでのHTMLエスケープのためのヘルパー関数です。現在ではhtml_escape()関数を使ってください。

何故脆弱性が作り込まれたのか?

→エスケープが漏れたから。

何故エスケープが漏れたのか?

→必要性が見落とされたため。

何故必要性が見落とされたのか?

→必要性の判断を誤った(必要なのに不要と判断した、あるいは、見逃した)ため。

何故必要性の判断を誤ったのか?

→必要性の判断が難しい、手間がかかるから。

何故必要性の判断が難しいのか?

→エスケープが必要かどうかをコードを遡って確認しないとわからないから。

とまあ、こんな感じでそもそも必要な箇所をエスケープするというアプローチが間違っていることがわかります。

何故必要な箇所をエスケープするというアプローチだったのか?

→CodeIgniterの書籍なので標準の機能をつかうべきと考えたが、CodeIgniterに自動エスケープの機能がなかったため。あるいは時間がなかったため。

ともかく、実際にエスケープ漏れが発生して脆弱性のあるコードが出荷されてしまったという事実を重く受け止める必要があります。

どうすればよかったのか?

エスケープ漏れをなくすには、エスケープが必要かどうかの判断をなくす、あるいは、判断を簡単にすることです。

また、エスケープしていない箇所を簡単に探せるようにすることも漏れを発見し防ぐためには役立つでしょう。

例えば、以下のような対策が考えられます。

  1. HTMLでの変数の出力をなくす
  2. HTMLでの変数の出力時にはすべて自動的にエスケープする

1.はHTMLに変数を埋め込むことを規約として禁止し、変数はAjaxでJSONで渡すとかです。

2.は

  • テンプレートエンジンを使いデフォルトでエスケープする
  • echoなど生で出力する命令の使用を規約として禁止して、専用の自動でエスケープする出力関数を作成し必ずそれを使い出力する

という方法が考えられます(ただし残存リスクあり)。

1.は制約としてきついので2.で対応する方が柔軟かと思います。CodeIgniterであれば例えばTwigを使うという方法があります。

ただし、2.の場合は、コンテクストに応じたエスケープをしないと脆弱性になるというリスクは残ります。

まとめ

  • HTMLに変数を出力する際に、手動でエスケープ関数を書くというコーディングは絶対に止めましょう。1か所忘れたらXSS脆弱性になる可能性があります。少なくともデフォルトでエスケープされるようにすべきです。

関連

Date: 2015/06/23

Tags: xss, security, codeigniter, book