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

Path traversal problem with two dynamic values #11

Closed
misuken-now opened this issue Aug 21, 2023 · 3 comments
Closed

Path traversal problem with two dynamic values #11

misuken-now opened this issue Aug 21, 2023 · 3 comments

Comments

@misuken-now
Copy link

misuken-now commented Aug 21, 2023

azuさん、ご無沙汰しております。
misuken です。以前は大変お世話になりました。

早速ですが本題です。
url-cheatsheetの内容も参考にしながら、パス部分が自動エンコードされるタイプのURL生成ライブラリ url-from を作成していた際に気付いたのですが、パストラバーサル問題に関しては encodeURIComponent() を使っても開発者が想定していないパスになる場合があるのと、その対処法についてです。

レアパターンとは思いますが以下のように複数の動的値を持つパターンです。

const name = "<user input>"
const tag = "<user input>"
const url = `https://example.com/user/${encodeURIComponent(name)}/${encodeURIComponent(tag)}`;

1つのときも1階層上のindexへのアクセスにはなっていたわけですが、複数になるとより自由度が生まれてきます。

nametag の値がそれぞれ ..admin だった場合、生成されるURLは https://example.com/user/../admin となり https://example.com/admin に誘導されます。
admin の後に何かパスやクエリを追加できるわけではないので害が出ることはないと思いますが、2つ以上の動的値が存在すると、開発者の意図しない階層のパスに誘導される可能性が出てきます。

.encodeURIComponent() ではエンコードされませんし、もし自力で %2E に変換したとしても、ブラウザでは正規化されて . と解釈されるようです。

url-fromではいくつかの理由によって安全性を評価し、開発者の意図しない階層のパスにアクセスされるよりは、当該条件の場合は半角スペースに置換する方法を取ることにしました。
https://github.com/misuken-now/url-from#path-traversal-protection

... が入力された場合の適切な対処法はどのようなものになるのでしょうかね?

以下は余談です。

wikipediaのURLを動的に生成する場合。

const name = "<user input>"
const url = `https://en.wikipedia.org/wiki/${encodeURIComponent(name)}`;

ブラウザで https://en.wikipedia.org/wiki/... というURLにアクセスすると、 https://en.wikipedia.org/wiki/Ellipsis にリダイレクトされますが、 https://en.wikipedia.org/wiki/.. にアクセスすると https://en.wikipedia.org/wiki でアクセスしたときと同様の https://en.wikipedia.org/wiki/Main_Page にリダイレクトされます。
curlで叩く場合は .%2E で結果が違うので、ブラウザのアドレスバーからの入力は正規化されてしまっていると解釈していますが、少なくとも ... はブラウザからはアクセスできないURLになっていそうです。

つまり、パス部分でタグ名を取扱う場合、 ... というタグ名を扱う方法が無いということでもありそうです。

@azu
Copy link
Owner

azu commented Aug 21, 2023

ありがとうございます。
.. の扱い難しいですね。次の記事で書かれている内容と近いと思います。

自分の知っている限りでは現状のURL系のウェブの仕様に .. をいい感じに扱ってるくれるものがない気がします。
そのため、.. だけのpart(nametag)を拒否するという実装になってるケースが多いのかなと思います。
(test.js のような.を含むケースはあり得なくはない。test..test はほぼあり得ないので .. を含む時点で弾いても大きくは問題ないが稀にあるかもしれない。..に完全一致はpath traversalでしか使わないので、弾いても問題ないという印象)

Honoであったパストラバーサルの修正では (?:^|\/)\.\.(?:$|\/) というのを拒否するようになっていました。
これは /../ or .. or /.. or ../ があったら弾くという実装ですね。

GitHubのリポジトリ名は . を含めることはできますが、.だけで構成されたリポジトリ名は拒否する実装になっているのも似たような理由から来ているのだと思います。

Aug-21-2023.23-07-36_optimized.mp4

/^\.+$/

@azu azu closed this as completed in 3760d8e Aug 21, 2023
@azu
Copy link
Owner

azu commented Aug 21, 2023

ちょっとまだ綺麗なパターンは思い付いてないので、とりあえず愚直な修正を入れました。
3760d8e

URLの仕様的には . and ..が特別なpath segmentとして定義されているため、この2つを拒否するというチェックの例を入れました。... は特別な意味を持たない。

@misuken-now
Copy link
Author

自分の知っている限りでは現状のURL系のウェブの仕様に .. をいい感じに扱ってるくれるものがない気がします。 そのため、.. だけのpart(nametag)を拒否するという実装になってるケースが多いのかなと思います。 (test.js のような.を含むケースはあり得なくはない。test..test はほぼあり得ないので .. を含む時点で弾いても大きくは問題ないが稀にあるかもしれない。..に完全一致はpath traversalでしか使わないので、弾いても問題ないという印象)

GitHubのcompareなんかは a..b a...b のように連続するドットを含んだりするので、バージョンや期間を取り扱うようなシステムでは「aからb」のような表現で使用されるかもしれません。

また、.. のみの場合は何かしらの対応を行う必要があるわけですが、どうやって弾くかも結構難しいポイントだったりします。

url-from の場合、例外とすることも検討しましたが、それはそれで開発者の望まない想定外の自体が起きるのでwarnを発生させつつ半角スペースに置換する方法を取りました。

開発者がURL生成機構を利用する際、一定のセキュリティ知識を持っている人でないと .. の問題を考慮することはありません。ユーザーの入力内容によって .. が設定されたときだけ例外が発生すると、他人のアクセス時にもそのデータを使用する作りではサイト全体が表示されなくなる可能性があります。

URL生成処理のために都度都度 try catch を書けば回避できますが、あまりに使用感が悪くなりますし、単純な埋め込みの際に .. の問題に気付いていない開発者が try catch を追加しようと思うわけがありません。

このように、弾き方によってもある意味攻撃のようなことが起きてしまうので、例外を投げるという選択肢も罠になると考えています。

url-from で採用したwarn & 半角スペース置換の方法は、ひとまずリスクが最も低いであろう半角スペースへの置換によって無害化しつつ、開発者に .. への対処を促す意味で warn を出し、それに気付いた開発者が .. が渡ってきている場所において .. をどのように扱うか自身で対応してもらうというものです。

例外とするか、何かに置換するか、エラー処理を書くか、入力段階でバリデーションを書くか、そのシステムにとってベストな対処法はライブラリ側では決められないため、開発者が気付くまで安全につなぐ方法が安全と使用感を両立させる選択肢になるかなと。

このあたりすごく色々考えたのですが、どうするか決めるのが非常に難しかったです。

Honoであったパストラバーサルの修正では (?:^|\/)\.\.(?:$|\/) というのを拒否するようになっていました。 これは /../ or .. or /.. or ../ があったら弾くという実装ですね。

getFilePath() の戻り値の型が string | undefined になっているのが面白いなと思いました。
もし "" 空文字を返すとそれはそれでまたリスクに繋がりそうですが、 undefined であればこの undefined を型的にどう扱うか開発者が検討する必要が出てくるからです。(対処法を開発者に委ねるという意味では url-from と同じ筋の良い方法だと思います)

ただ、開発者がなぜこの関数が undefined が返ってくる可能性があるのか不思議に思うはずなのと、一定の水準に満たない開発者は安易に getFilePath(options) || ""getFilePath(options) as string のようなことをしてしまうことが想像できるので、.. が渡ってきて突然システムが止まるなどのトラブルに直面して初めて気付く流れになりそうな気もします。

このあたり、人がどのように行動するかまで考えて設計する必要があるのが .. の厄介なポイントだと感じています。

GitHubのリポジトリ名は . を含めることはできますが、.だけで構成されたリポジトリ名は拒否する実装になっているのも似たような理由から来ているのだと思います。

/^\.+$/

まさにそこを意図した実装に見えますね。

... は作れるので /^\.{1,2}$/ が正しそうですね。

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

No branches or pull requests

2 participants