プログラマ専用SNS ミクプラ

勉強なう さんが書きました: ③上の２つ以外に脆弱性はありますか？

勉強なう さんが書きました:その脆弱性によって攻撃されてしまうのでしょうか？
②はわからないでしょうか？

勉強なう さんが書きました: その脆弱性によって攻撃されてしまうのでしょうか？

勉強なう さんが書きました: ②はわからないでしょうか？

かずま さんが書きました:

勉強なう さんが書きました: その脆弱性によって攻撃されてしまうのでしょうか？

このプログラムが正常に終了しないことも脆弱性の一つでしょう。
攻撃とはどういうことを言っていますか？
システムを落とすこと？
読めないファイルを読みだすこと？

仮にこのプログラムが root の権限で動いていたとしましょう。
このプログラムのパスワードを知っているユーザが MSG: に対して
; shutdown -h now; echo abc
と入力したら
system("echo [user]; shutdown -h now; echo abc >>/tmp/memo.log");
でシステムが落ちます。

ユーザ入力をそのまま system() で実行するのは危険です。

ユーザ入力をそのまま printf() の書式に使うのも脆弱性の一つです。
USER: に対して %s%s%s と入力したら、
printf("NG(%s%s%s)\n"); を実行して、おかしなことになります。

勉強なう さんが書きました: ②はわからないでしょうか？

gets のバッファオーバーフローなら、スタック上にある get_line() の
リターンアドレスを壊してどこに飛んでいくかわからない。
バッファオーバフローで書き込んだところに飛ばせば、
そこに書かれた攻撃者のプログラムを実行できます。

最初の get_line() の gets() で EOF だと、passwd は 未初期化の buf
のコピーを指しますが、こちらは被害は少ないでしょう。

勉強なう さんが書きました:貴方の挙げた2つについて②をいいますと、
それぞれUSER、PASS、MSGが表示された時、256バイト以上入力することが攻撃にあたりますね。

＞＞２４行目の方も256バイト以上の入力でいいんでしょうか？

24  if(strcmp(HASH, crypt(passwd, SALT)) == 0){

へにっくす さんが書きました:

勉強なう さんが書きました:貴方の挙げた2つについて②をいいますと、
それぞれUSER、PASS、MSGが表示された時、256バイト以上入力することが攻撃にあたりますね。

＞＞２４行目の方も256バイト以上の入力でいいんでしょうか？

初期化していないとはどの変数ですかね。

コード:

24  if(strcmp(HASH, crypt(passwd, SALT)) == 0){

HASH、SALTは定数ですしpasswdはその前のget_line()でセットしています。
なので24行目は「変数が初期化していない」とは言えないと思います。
(かずまさんがpasswdについて述べられていますが、結局はget_line関数の中の話ですので)
修正方法はまず自分で考えてみてください(質問内容には書かれていないので)。
たとえばget_line関数で使用しているgetsはVC++ならgets_sに置き換えるとかね(もちろんこれだけではだめですよ？)。

勉強なう さんが書きました:HASHとSALTは変数じゃないんでしょうか？

24  if(strcmp("$1$54lt$""4OOZtZMXwyHi.bwq2QS6U0", crypt(passwd, "$1$54lt$")) == 0){

勉強なう さんが書きました:get_lineについて調べましたが、自分ではわかりそうにないので、できればどこをどのように修正するか教えてもらえないでしょうか？

char *get_line()
{
  char buf[256]={0}; // 初期化を忘れずに(初期化されてないと質問内容にあったのでここぐらいは答えてほしかったなあ^^;)
  gets_s(buf, sizeof(buf) - 1); // これで256文字以上入力すると、CRTが無効な引数を検出する。
  return strdup(buf); // 入力していない場合は空文字列の複製が返る
}

へにっくす さんが書きました:

勉強なう さんが書きました:HASHとSALTは変数じゃないんでしょうか？

6行目と7行目で#defineで定義してますよね？
こういうのは変数でなく「定数」です。
C言語のdefineについて
コンパイルするときに#defineで定義したものは置換しますので、実際にはこう書いているのと同じです。

コード:

24  if(strcmp("$1$54lt$""4OOZtZMXwyHi.bwq2QS6U0", crypt(passwd, "$1$54lt$")) == 0){

勉強なう さんが書きました:get_lineについて調べましたが、自分ではわかりそうにないので、できればどこをどのように修正するか教えてもらえないでしょうか？

こんな感じになるかな(あくまでも一例です)

コード:

char *get_line()
{
  char buf[256]={0}; // 初期化を忘れずに(初期化されてないと質問内容にあったのでここぐらいは答えてほしかったなあ^^;)
  gets_s(buf, sizeof(buf) - 1); // これで256文字以上入力すると、CRTが無効な引数を検出する。
  return strdup(buf); // 入力していない場合は空文字列の複製が返る
}

あとはCRTが無効な引数を検出した場合に呼び出す関数を設定します。
_set_invalid_parameter_handler - MSDN

勉強なう さんが書きました:①fgets関数などでもふせげるでしょうか？
②ということはこのコードの脆弱性は11行目のバッファオーバーフローの脆弱性だけということですか？
　ほかにもあればその部分と修正方法、どのように攻撃されてしまうのか教えてくださいorz

fgets(buf, sizeof(buf) - 1,stdin);