テンプレート変数へ中国語を入力できない($modx->db->escapeの問題?)

プログラム(機能)関連の開発の話題
shobu
メンバー
メンバー
記事: 91
登録日時: 2011年5月26日(木) 16:54

Re: テンプレート変数へ中国語を入力できない($modx->db->escapeの問題?)

投稿記事by shobu » 2011年5月27日(金) 14:35

yama さんが書きました:
shobu さんが書きました:未だキャッシュの方の問題が理解できていないのですが、modx->db->escape内の問題が解決出来れば、こちらも解決、ということではないのでしょうか?

ここで$modx->db->escape()を使う必要自体、目的を考えるとどうなのかな?と思います。データベース関係ないはずですし。必要なエスケープを行なわないために問題が起きる可能性はありますが、今のところ実例は確認できてないので検証を後回しにしています。
以前のMODXは、この部分の情報が化けたまま動いてました。「チ¥ャ¥ン¥ク」みたいな状態になってても、基本的に化けた情報をベースにやりとりしてるから結果オーライで動いてることが多かったみたいですね。チ¥ャ¥ン¥クという名前のチャンクを呼ぶ時点で、チ¥ャ¥ン¥クとして呼んでました。
そもそも脆弱性対策のためのmysql_real_escape_stringなので、せっかくエスケープしてるのに全然意味がないことになります。


この辺の処理をじっくり見ていないので何とも分かりませんが、最後はファイルに書き出して終わるわけで、意味はないように見えますね。

勝手な想像では
    そのうち、または元々は、キャッシュデータをDBに格納しようと考えていたときの名残
    チャンク名等も状況によってはエスケープ済みの状態でDBに格納されているので、念のため(なのかよく分からないが)、配列のキー名もそろえておこうかなぁ
など。。これは本家の担当に確認しないと意図は分からないのでは・・・・。

ちょっと気になったのは、私がちょっとしつこい気もしますが、
yama さんが書きました:以前のMODXは、この部分の情報が化けたまま動いてました。

のところで、確かにその現象は起こりえたのですが、それはサーバ側の設定に問題があり、ちゃんと設定できれば解決出来ていたし、他のCMSなどでも同じ現象が起きていたので、MODxが・・・というよりはphp,mysqlといったサーバ側の問題という認識でした。
問題が解決出来ていれば化けたまま運用ということも無いので、今になってここに手が入るとは思っていなかったという認識です。
(で、今回の元トピの状況にビックリしたという経緯)。
以前は文字化けの件がよく話題に上っていたと思いますが、最近少なくなったのはこの辺の問題が認知されて正しい設定が出来るようになってきたからだと思っていたのですが・・・。どうなんでしょうかね。。。

それではー。
アバター
yama
管理人
記事: 3096
登録日時: 2009年7月29日(水) 02:50

Re: テンプレート変数へ中国語を入力できない($modx->db->escapeの問題?)

投稿記事by yama » 2011年5月27日(金) 15:17

shobu さんが書きました:そういうことではなく、私の環境のように全てutf8でやりとりできていれば「内部でやりとりする情報」も問題なく、そうではないと問題が出る(それもエスケープ周り?)ということでしょうか。

ということになりますね。内部でやりとりする情報は、問題がある場合はキャッシュを見れば分かります。エスケープずれが発生している場合は、盛大に化けています。ただ、化けるのは変数名のほうであって変数の値は化けないので、表面的にはちゃんと使えているように見えることが多いと思います。変数名が化けると、当然ながらシステム的には問題が発生しやすくなります。

shobu さんが書きました:そうなると、あとは・・・
    前述の代替コードの妥当性
    インストール時点での不具合環境のチェック方法
の確認ですね。まだ、実験できていませんが、こちらで見てみたいと思います。

代替えコードの検証は、結果をざっと見る程度にはなると思いますが、あまり難しくないと思います。少なくとも、現状よりは改善されると思いますし、そういう意味では気楽だと思います。この代替えコードの精度がしっかりしていれば、インストール時点のチェックも不要だと思います。mysql_set_charsetが使えるかどうかを判定するだけで、このコードの使用の可否を分けてよいと思いますので。

というわけで、うまくいけばワンブロック書き換えるだけであっさり対応できそうですね。

shobu さんが書きました:
    そのうち、または元々は、キャッシュデータをDBに格納しようと考えていたときの名残
    チャンク名等も状況によってはエスケープ済みの状態でDBに格納されているので、念のため(なのかよく分からないが)、配列のキー名もそろえておこうかなぁ
など。。これは本家の担当に確認しないと意図は分からないのでは・・・・。

「エスケープ済みの状態でDBに格納されている」ってことはないですよ。だから分かりにくいのですが。エスケープが必要なのはSQLコマンドとしてデータベースにアクセスする時だけであって、実際に値が格納される瞬間にはエスケープがほどかれた状態に戻ります。このへんのコードが書かれたのは、MySQL4.0が全盛の、DBAPIのエスケープ処理が原始的だった頃の実装なので、同じ処理がキャッシュ生成でも流用できると考えたのでしょう。理由は何にせよ、あまりいいアプローチではないです。
今回の改善でこの部分も影響を受けますので、もしそれで問題が発生する場合は、この機会にキャッシュ専用のエスケープ処理を書いてもいいんじゃないかなと思ってます。変数名として使ってはいけない文字を切り分けてエスケープするだけなので、理屈としてはあまり難しくなさそうです。

追記
エスケープ済みの状態同士で意図的に照合させようとした可能性はありますが、ちょっとヤッツケ過ぎだと思いますw
shobu
メンバー
メンバー
記事: 91
登録日時: 2011年5月26日(木) 16:54

Re: テンプレート変数へ中国語を入力できない($modx->db->escapeの問題?)

投稿記事by shobu » 2011年5月27日(金) 15:31

何度もおつきあいありがとうございます。VirtualPC用のCentOSイメージで信用できそうなのがないかなと探しているのですが、見あたらないのでVMWareで行くか、1からインストールするか・・・と関係ないところで詰まってます(笑。

yama さんが書きました:代替えコードの検証は、結果をざっと見る程度にはなると思いますが、あまり難しくないと思います。少なくとも、現状よりは改善されると思いますし、そういう意味では気楽だと思います。この代替えコードの精度がしっかりしていれば、インストール時点のチェックも不要だと思います。mysql_set_charsetが使えるかどうかを判定するだけで、このコードの使用の可否を分けてよいと思いますので。

というわけで、うまくいけばワンブロック書き換えるだけであっさり対応できそうですね。


代替コードに現時点で問題がないならば、現時点ではmysql_set_charsetが無ければ全て置き換え・・・でもよさそうですが、例えばrhel4,5系で本来問題がない環境でも代替コードを使用することになると思います。
先々、何か別の問題が発見されてreal_escapeに手が入る可能性(php側というよりmysql側のlibmysqlclientでしょうかね)も考えると代替にするのは最後の手段的なアプローチの方が本家でも取り込みやすいのではないでしょうか。
状況確認が必要なのでインストーラ的には工数が増えますが。

yama さんが書きました:「エスケープ済みの状態でDBに格納されている」ってことはないですよ。だから分かりにくいのですが。エスケープが必要なのはSQLコマンドとしてデータベースにアクセスする時だけであって、実際に値が格納される瞬間にはエスケープがほどかれた状態に戻ります。このへんのコードが書かれたのは、MySQL4.0が全盛の、DBAPIのエスケープ処理が原始的だった頃の実装なので、同じ処理がキャッシュ生成でも流用できると考えたのでしょう。理由は何にせよ、あまりいいアプローチではないです。
今回の改善でこの部分も影響を受けますので、もしそれで問題が発生する場合は、この機会にキャッシュ専用のエスケープ処理を書いてもいいんじゃないかなと思ってます。変数名として使ってはいけない文字を切り分けてエスケープするだけなので、理屈としてはあまり難しくなさそうです。

なるほど・・・。0.9.6あたりだとrealの付かないエスケープの方を利用していますね。この辺の経緯は分かりませんが、上記の代替コード処理などで問題が縮小すれば、確かに冗長で無駄なコードなのかもしれませんが、本家のコードからわざわざ離れなくても良いようにも思えます。本家の方に別の正しい形でのエスケープ処理を提案して採用して貰う方向性はないでしょうか?
アバター
yama
管理人
記事: 3096
登録日時: 2009年7月29日(水) 02:50

Re: テンプレート変数へ中国語を入力できない($modx->db->escapeの問題?)

投稿記事by yama » 2011年5月27日(金) 15:39

shobu さんが書きました:本家の方に別の正しい形でのエスケープ処理を提案して採用して貰う方向性はないでしょうか?

実はそれを書く担当が私になってるので、私が何もしないと何も変わらないですw

コミットリーダーも問題を認識していて、このままでよいとは思ってないです。
でも正直難しいですね。今回の件で解決できれば、今まで悩んでたのは何だったんだ的な感じで嬉しいですが。
夏には1.0.6がリリースされると思うので、それまでに解決できるとよいなと思ってます。
shobu
メンバー
メンバー
記事: 91
登録日時: 2011年5月26日(木) 16:54

Re: テンプレート変数へ中国語を入力できない($modx->db->escapeの問題?)

投稿記事by shobu » 2011年5月27日(金) 16:51

yama さんが書きました:
shobu さんが書きました:本家の方に別の正しい形でのエスケープ処理を提案して採用して貰う方向性はないでしょうか?

実はそれを書く担当が私になってるので、私が何もしないと何も変わらないですw

コミットリーダーも問題を認識していて、このままでよいとは思ってないです。
でも正直難しいですね。今回の件で解決できれば、今まで悩んでたのは何だったんだ的な感じで嬉しいですが。
夏には1.0.6がリリースされると思うので、それまでに解決できるとよいなと思ってます。


まずは私の方は代替escape処理の件とインストーラでの状況確認について調査してみます。

キャッシュファイル処理中のエスケープは別の処理で置き換えられる可能性が高そうなので、最終的には元トピの件とキャッシュファイル処理の件は分けて考えて良いですよね?

それでは、引き続きよろしくお願いいたします。
アバター
yama
管理人
記事: 3096
登録日時: 2009年7月29日(水) 02:50

Re: テンプレート変数へ中国語を入力できない($modx->db->escapeの問題?)

投稿記事by yama » 2011年5月28日(土) 00:55

shobu さんが書きました:キャッシュファイル処理中のエスケープは別の処理で置き換えられる可能性が高そうなので、最終的には元トピの件とキャッシュファイル処理の件は分けて考えて良いですよね?

ですね。キャッシュのほうは変数名としては意味を持つ「$」をエスケープしてなかったりするので、まあやっぱり別だなあという感じがします。そんな文字をわざわざ使う人はなかなかいないとは思いますが。

たぶん代替のescape処理に差し替えるだけであっさり解決するんじゃないかという気がしています。接続側の文字セットを確定できるからmysql_real_escape_stringを使う意味があるのですが、それだったらCMSのシステム側で分かりますし(・・ということですよね、たぶん)。今やってる処理も、いったんシステム側でeuc-jpに落としているわけですが、utf-8であることがはっきりしていて、そのutf-8に対して問題なくエスケープを行なえる処理を実装すれば、それで解決としていいんじゃないかと思います。
まずはテストで確かめてみたいですね。
shobu
メンバー
メンバー
記事: 91
登録日時: 2011年5月26日(木) 16:54

Re: テンプレート変数へ中国語を入力できない($modx->db->escapeの問題?)

投稿記事by shobu » 2011年5月29日(日) 02:05

まだ、テスト環境ができていないので本件のテストはまだなのですが、元トピの通り、問題がテンプレート変数のみでpagetitleやintroxextに及ばないのがなぜか、という点が気になって少しコードを眺めたのですが、気になる点に行き当たりました。

pagetitleやintroxextは save_content.processor.php のだいぶ上の方でPOSTから変数に保存されるのですが、一応、$modx->db->escapeの処理はされています。
しかし、この段階ではまだ mysqlへのconnectが確立していないため、 $modx->db->escape内の分岐では一番最後を通ることになります。
(ここはconnectの件含め、確認済みです)

コード: 全て選択

   function escape($s) {
//      if (function_exists('mysql_set_charset') && $this->conn)
      if (function_exists('mysql_real_escape_string') && $this->conn)
      {
         $s = mysql_real_escape_string($s, $this->conn);
      }
      elseif ($this->config['charset']=='utf8' && $this->conn)
      {
         $s = mb_convert_encoding($s, 'eucjp-win', 'utf-8');
         $s = mysql_real_escape_string($s, $this->conn);
         $s = mb_convert_encoding($s, 'utf-8', 'eucjp-win');
      }
      else
      {
         $s = mysql_escape_string($s);
      }
      return $s;
   }

つまり mysql_escape_string を使用します。この関数が一応エスケープ処理してくれますし、エスケープ対象はrealと同じようなので脆弱性を抱える可能性は低いと思いますが、関数自体は DEPRECATED となっているようです(なくなるときが来る?)。

この関数が非推奨である理由がrealと違って「カレントの文字セットを考慮したエスケープ」が無いからという点だけかどうかはわかりませんが、それだけだとすればUTF-8前提ならば代替コードの代わりにこの関数を使ってしまうのもひとつの手段かな・・・と一瞬考えました。
yama さんが書きました:たぶん代替のescape処理に差し替えるだけであっさり解決するんじゃないかという気がしています。接続側の文字セットを確定できるからmysql_real_escape_stringを使う意味があるのですが、それだったらCMSのシステム側で分かりますし(・・ということですよね、たぶん)。今やってる処理も、いったんシステム側でeuc-jpに落としているわけですが、utf-8であることがはっきりしていて、そのutf-8に対して問題なくエスケープを行なえる処理を実装すれば、それで解決としていいんじゃないかと思います。

まぁ、こちらのコメントに沿っているといえば沿っているのかも・・・。

でも、やはり DEPRECATED となっていますし、関数内部に問題があったとしても今後修正されることはないとすれば、よくありませんよね。
となると逆にここの処理は念のため、例の代替コードで置き換えるべきだと思います。ただ、この部分は後述の理由により本来通過すべきではないと思います。

まず、realの方はlibmysqlclientの処理を呼び出して使用してくれるのだと理解していますが、これによって今後発生しうるmysql側の何らかの変更(脆弱性や仕様変更など)によってエスケープすべき対象や処理が変更になった時にphp側と切り離した形で対処できるので(逆に今はそのせいで文字コードを制御できないケースがあるわけですが)これを利用すべきということですよね。。。

そうなると、本来はまずconnectの確立を最初に行い、pagetitleなどもrealの方できっちりエスケープした上でDB投入されるべきと思われます(これがどうでもいいならば、逆に全部でrealを使う必要もないよね、、、realがついてるの片っ端からとっちゃえばUTF8なら一応今回の問題はなくなるね、というわけです)。
記事編集は管理画面で管理者のみが入力すると思いきや、サイトの作りやスニペット次第でウェブユーザに開放できる以上、しっかり行わないといけない部分だと思います。

よって、connectの確率が最初にされていれば、例のエスケープルーチンで現在 mysql_escape_string を通過する所は通過することがなくなり、基本的にはreal付きのエスケープを使用し、DB環境に問題がある場合のみ代替コードを通過する感じになる・・・はずなのですが、例のキャッシュ処理でのエスケープや他のルーチンでも同じ事をしていると、connect無しにこのエスケープルーチンに入ってくる可能性もありそうなので、connect無し時の mysql_espace_string は代替コードに置き換えて置いておくか、 connectあり&DB問題なしならばrealエスケープ、それ以外は代替コードとなるのかな。。。
キャッシュ処理時のconnect状況がどうなっているかは確認していませんが、今回の発端の改修の原因だとすれば、connectありでrealを通過ですね。。。
全然別のところで、似たような必要ないエスケープをしている部分が潜んでそうですが。。。

少々元トピの件から脱線気味ですが、本件も同時に解決できればいいかなぁと思います。save_content.processor.php の件は既知の問題でしょうか?この管轄もyamaさんだったら話が早そうですが、、、。