というタイトルのQiitaの記事を見かけ、いったいどれほどダメなコードだったのかと興味本位でコードを眺めてみました。
漠然と眺めたのでさくっとコード本体から目を通したわけです。
見た瞬間に飲んでいたお茶を噴き出してしまいました。
select * from usersを投げてる時点で頭を抱えて、総当たりforループで吐き気がし、account.password === passwordで「平文！？」と叫んで壁を殴りつけたくなりました。インフラ屋なので、まずパフォーマンスに目が行った感じです。
元ネタのTwitter側で騒いでいますけれど、if “true” === “true”は哲学的ではあるものの、致命的な問題ではありませんので、自分はまだまだ、まあ目をつぶれます。ロジック的には破綻していませんし、大きな問題は別にありすぎて、どうでもいい感じになっています。
そして、if (‘#login’).clickのところに差し掛かった瞬間に、「あれ？ ちょっとまってこれJS！？」となって先頭行に飛び、SCRIPTの文字を見た瞬間に椅子から転げ落ちて床で死にました。apiService.sqlが危険すぎます、SQL文を渡している相手なので、dropやtruncate投げ放題の可能性さえあります。
まあ、SQLのログインアカウントがハードコートされていてきちんとselect限定になってるといいですね。情報漏洩はしますがデータロストはしないかもしれませんから。
まあ、それでもサーバサイドで動いていたとしてもCookieがザル過ぎてこんなのどうしようもありませんが。
こんなひどいコード見たことがない。というか認証をクライアントにやらせるとか何をどう考えたらそうなったんでしょうか。
インフラがいくらセキュリティを頑張ったって、こんなことされたらなんにもなりません。
アプリケーションが自らばらまいているうえにトロイの木馬になってくれているんですから。
そして恐ろしいことにTwitterやQiitaのコメントにこの恐ろしさがわからないコーダーやPGがいるのだ、ということが何よりも恐ろしく感じました。
これがNodeJSだったというならまだましになるわけですが、どこからどう見てもクライアント側コードです。
たとえこれがイントラで動いているものだとしても、許容できるものではありません。
恐ろしいコードでした…。
最後に一行目のコメントを改めて読み、「そうじゃない、そうじゃないんだ…」と力なく呟くことしかできませんでした。
正直、運用サイドとしてみた場合、このコードは笑いごとではありません。これを笑う、というのは実装段階までだけを考えた場合で、これが運用ステージに入ったと考えると笑う余力などあるわけもなく、ただただ恐怖です。
システム停止許可すら連絡するのももどかしいレベルで、即座にWebサーバとDBの緊急停止を即座に実行するレベルですね。
一ヶ月で復旧できるといいですね。RootKit突っ込まれる余地がふんだんにあるので、インフラ含めてすべてを再構築しなおすべきです。
