OWASP ZAPの自動スキャン結果の分析と対策：オープンリダイレクト

<?php
  require_once './common.php';
  if (! isset($_POST['userid']) || ! isset($_POST['pwd']) || ! isset($_POST['url'])) {
    exit;
  }
  try {
    $dbh = dblogin();
    $userid = filter_input(INPUT_POST, 'userid');
    $pwd = substr($_POST['pwd'], 0, 6);
    $url = filter_input(INPUT_POST, 'url');

    $sql = $dbh->prepare("SELECT id, userid FROM users WHERE userid = ?");
    $sql->bindValue(1, $userid, PDO::PARAM_STR);
    $sql->execute();
    $row = $sql->fetch(PDO::FETCH_ASSOC);
    $sql = null;
    if (! empty($row)) {
      $sqlstm = $dbh->prepare("SELECT id, userid, super FROM users WHERE userid = ? AND pwd = ?");
      $sqlstm->bindValue(1, $userid, PDO::PARAM_STR);
      $sqlstm->bindValue(2, $pwd, PDO::PARAM_STR);
      $sqlstm->execute();
      $row = $sqlstm->fetch(PDO::FETCH_ASSOC);
      if (! empty($row)) {
        error_log("row[id]=" . $row['id'] . ", row[super]=" . $row['super']);
        $_SESSION['login'] = true;
        $user = new User($row['id'], $userid, $row['super']);
        setcookie('USER', serialize($user), 0, '/');
        header('Location: ' . $url . '?' . SID);
      } else {
        e("パスワードが違います");
        exit;
      }
    } else {
      e("そのユーザーは登録されていません");
      exit;
    }
  } catch (PDOException $e) {
    die('接続に失敗しました: ' . $e->getMessage());
  }
?>

logindo.php でソースを検索したところ、login.php だけがヒットしました。login.php では、入力として URL を渡された場合に、logindo.php にその URL を渡しています。つまり、login.php についても調べる必要があります。

login.php を検索したところ、4件ヒットしました。3件は、URL として、todolist.php を固定で渡していましたが、1件（common.php）は、別の URL を指定できそうな感じです。

例えば、ログアウトした状態で、インポートに遷移すると、「ログアウトしています。ログイン（リンク）」という画面になりますが、このログインをクリックしたときは、URL が import.php となり、ログインした後、インポートに遷移する仕様のようです。

というわけで、リダイレクト先が変化するということになります。リダイレクト先が変化しないなら、hiddenパラメータを削除し、リダイレクト先を固定する対策で良かったと思います。

今回はリダイレクト先が変化するので、別の対策を行う必要があります。徳丸本では、ページを番号管理にして URL を直接指定させない方法と、リダイレクト先のドメイン名をチェックする方法を説明してくれています。

今回の対策としては、前者のページを番号管理にする方法は修正量が多いので、後者のリダイレクト先のドメイン名をチェックする方法にしようと思います。

オープンリダイレクトの脆弱性の再現

実際にオープンリダイレクトの脆弱性を再現させていきます。

いつもなら、Burp Suite を起動するところですが、今回は OWASP ZAP を使います。

ログアウトしておき、ログイン画面に遷移させます。普通に id とパスワードを入力します。OWASP ZAP で、ブレークポイントを有効にしておき、ログインボタンをクリックします。すると、POSTリクエストがブラウザに送信される前に OWASP ZAP が止めてくれます。ここで、データの url を todolist.php から http://32637559960979135.owasp.org に改変します。

すると、本来は、ログインすると、Bad Todo の一覧に遷移するところが、以下のように外部サイトに遷移しようとしました。オープンリダイレクトの脆弱性が再現できました。

オープンリダイレクトの脆弱性の対策

では、オープンリダイレクトの脆弱性に対策を行います。

logindo.php の $url に入るのは、固定で todolist.php か、以下の common.php の $current です。

<?php
function require_loggedin() {
  if (! is_loggedin()) {
    $user = new User();
    $current = $_SERVER['PHP_SELF'];
    $title = "ログアウトしています";
    $content = "ログアウトしています。<a href='login.php?url=$current'>ログイン</a>";
    require "template.php";
    exit;
  }
}

この $_SERVER['PHP_SELF'] には、例えば、/todo/import.php とか、/todo/export.php が入ります。

つまり、先頭に http が付くことは無いです。また、http を使わずに、www.xxx.com のような攻撃をしてみましたが、この場合は、https://example.jp/todo/www.xxx.com? にアクセスする動作となりました。

よって、先頭に http が付いてる場合はエラーとして、todolist.php に飛ばすような対策としたいと思います。以下が対策対象のコードです。

<?php
// 対策前
header('Location: ' . $url . '?' . SID);

次のように、http で始まる場合は、強制的に $url を todolist.php に設定します。また、その下のところは、エスケープ処理が無いので、htmlspecialchars関数を使っています。

<?php
// 対策後
if (preg_match('/^http/', $url) === 1) {
  $url = "todolist.php";
}
header('Location: ' . htmlspecialchars($url) . '?' . SID);

これで、同じ攻撃を行ったとき、外部サイトではなく、一覧のページに遷移することが確認できました。

logindo.php の全体の修正点を貼っておきます。

--- todo.org/logindo.php        2018-08-15 15:29:23.000000000 +0900
+++ todo.change/logindo.php     2024-08-15 21:00:24.000000000 +0900
@@ -1,24 +1,34 @@
 <?php
   require_once './common.php';
+  if (! isset($_POST['userid']) || ! isset($_POST['pwd']) || ! isset($_POST['url'])) {
+    exit;
+  }
   try {
     $dbh = dblogin();
     $userid = filter_input(INPUT_POST, 'userid');
     $pwd = substr($_POST['pwd'], 0, 6);
     $url = filter_input(INPUT_POST, 'url');

-    $sql = "SELECT id, userid FROM users WHERE userid='$userid'";
-    $sth = $dbh->query($sql);
-    $row = $sth->fetch(PDO::FETCH_ASSOC);
-    $sth = null;
+    $sql = $dbh->prepare("SELECT id, userid FROM users WHERE userid = ?");
+    $sql->bindValue(1, $userid, PDO::PARAM_STR);
+    $sql->execute();
+    $row = $sql->fetch(PDO::FETCH_ASSOC);
+    $sql = null;
     if (! empty($row)) {
-      $sqlstm = "SELECT id, userid, super FROM users WHERE userid='$userid' AND pwd='$pwd'";
-      $sth = $dbh->query($sqlstm);
-      $row = $sth->fetch(PDO::FETCH_ASSOC);
+      $sqlstm = $dbh->prepare("SELECT id, userid, super FROM users WHERE userid = ? AND pwd = ?");
+      $sqlstm->bindValue(1, $userid, PDO::PARAM_STR);
+      $sqlstm->bindValue(2, $pwd, PDO::PARAM_STR);
+      $sqlstm->execute();
+      $row = $sqlstm->fetch(PDO::FETCH_ASSOC);
       if (! empty($row)) {
+        error_log("row[id]=" . $row['id'] . ", row[super]=" . $row['super']);
         $_SESSION['login'] = true;
         $user = new User($row['id'], $userid, $row['super']);
         setcookie('USER', serialize($user), 0, '/');
-        header('Location: ' . $url . '?' . SID);
+        if (preg_match('/^http/', $url) === 1) {
+          $url = "todolist.php";
+        }
+        header('Location: ' . htmlspecialchars($url) . '?' . SID);
       } else {
         e("パスワードが違います");
         exit;