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

APIが違うユーザー扱いになっている #4705

Closed
YuzuRyo61 opened this Issue Apr 15, 2019 · 15 comments

Comments

Projects
None yet
4 participants
@YuzuRyo61
Copy link
Contributor

YuzuRyo61 commented Apr 15, 2019

💡 Summary

ハッシュ化したトークンを扱ってAPI操作をすると違うユーザーとしてAPI操作してしまう。

📝 Steps to Reproduce

  1. app/create でアプリを作成
  2. 作成したシークレットキーでセッショントークンを作成 (auth/session/generate)
  3. ブラウザで認証してアクセストークンを発行する(auth/session/userkey) <=ここまでは自分のアカウントだということがわかった
  4. アクセストークンとシークレットキーをsha256化してハッシュ化
  5. ↑のキーを使って何かしらのAPI操作をする
  6. 自分のアカウントではなく他のアカウントとして操作していることになってる

※ノート削除とか一部機能は通じない???

@syuilo

This comment has been minimized.

Copy link
Owner

syuilo commented Apr 15, 2019

その「ほかのアカウント」は常に同じアカウントですか?

@YuzuRyo61

This comment has been minimized.

Copy link
Contributor Author

YuzuRyo61 commented Apr 15, 2019

その「ほかのアカウント」は常に同じアカウントですか?

どうやらしばらく置いてたりするとアカウントが変わっているようで・・・。

image

@syuilo

This comment has been minimized.

Copy link
Owner

syuilo commented Apr 15, 2019

ありがとうございます🙏

@syuilo

This comment has been minimized.

Copy link
Owner

syuilo commented Apr 15, 2019

たぶんどこかのクエリが抜けていて、認証したアカウントではなく常に最新のアカウントを持ってきているように思える

@syuilo

This comment has been minimized.

Copy link
Owner

syuilo commented Apr 15, 2019

でも見た感じ問題ないので謎

@YuzuRyo61

This comment has been minimized.

Copy link
Contributor Author

YuzuRyo61 commented Apr 15, 2019

(これは放おっておけない致命的なバグですね・・・。)

@acid-chicken

This comment has been minimized.

Copy link
Collaborator

acid-chicken commented Apr 15, 2019

一旦リリース消したほうがいいのでは。原因がわからないまま本リリース(非プレリリース)で出したままにしておくのは、脆弱性を把握していない運用者やユーザーを非常に困惑させると思います。

@syuilo

This comment has been minimized.

Copy link
Owner

syuilo commented Apr 15, 2019

テーブルの定義に問題がありそう

@syuilo

This comment has been minimized.

Copy link
Owner

syuilo commented Apr 15, 2019

これかな
typeorm/typeorm#1795

@syuilo syuilo closed this in 400cdf0 Apr 15, 2019

@syuilo

This comment has been minimized.

Copy link
Owner

syuilo commented Apr 15, 2019

リリースって消せるのかな?

@syuilo syuilo removed the label Apr 15, 2019

@acid-chicken

This comment has been minimized.

Copy link
Collaborator

acid-chicken commented Apr 15, 2019

今一度脆弱性がないか確認したあとv11を放棄してv12でやり直すのも選択肢の一つかも。(PHP 6 などのような事例)

@syuilo

This comment has been minimized.

Copy link
Owner

syuilo commented Apr 15, 2019

とりあえず 11.1.3 で出します

@mei23

This comment has been minimized.

Copy link
Collaborator

mei23 commented Apr 15, 2019

400cdf0
この変更を適用するのに、スキーマの同期とかマイグレーションは必要になりますかね?

@syuilo

This comment has been minimized.

Copy link
Owner

syuilo commented Apr 15, 2019

そう考えてます
なのでアクセストークン作成はできなくなってるかもしれないです
どうやって同期させるか考え中です

@syuilo

This comment has been minimized.

Copy link
Owner

syuilo commented Apr 15, 2019

TypeORM内部の実装に由来する問題だとしたら、もしかしたらスキーマの同期しなくても大丈夫かもしれないですね

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.