「sudoに10年以上前から存在した特権昇格の脆弱性が修正される」記事へのコメント

直ってんのか不安になる直し方

[Anonymous Coward]

それ直したっていうコミット [www.sudo.ws]とその部分のソース [www.sudo.ws]見てみたんだけど、
一瞬直し損なったのかと空目するような直し方だった。
多分肝心なのは

+ cp = buf;

だと思うので、
writeがコケる状態で文字を入れてはsudo_term_killを入力してleftとcpを乖離させる攻撃だったのかなと思うのだが、
write失敗でbreakしても入力ループ自体抜けてしまい問題の少なそうなsudo_term_eraseではbreakを消して、
問題の箇所ではwrite失敗を考慮しなければ無意味に見える処理の追加で対処している。

さらにコピペされているコードの片割れだけいじった事で不整合も起こしている。

とりあえずは良いとしてもこの調子で大丈夫なのかコレ……

Re:

[Anonymous Coward]

そうじゃないでしょ。

コピペ元（って便宜上呼ぶけど）のbreakは直近のwhile (cp > buf) の{}から抜けるために使ってるから、全く正しい。ところが、コピペ先の{}はwhileじゃなくてifだから、ここでbreakを呼ぶと大元のwhile(--left)から抜けてしまう。読み込んでないけど、修正された差分をみると、これは完全にやらかしてたってことだよ。コピペまでして対称的に書いてるつもりなのに全く違う動作になるから、気付いたら青ざめる系のうっかりだ。

#インクリメントの前置後置で笑ってしまう。

Re:

[Anonymous Coward]

しかしみじけ〜のにひで〜コードだな
cp > bufとかcp--一箇所にしろよと…

Re:

[Anonymous Coward]

簡潔に書こうとしてるから余計酷いって気もする。

Re:

[Anonymous Coward]

バッファ中の位置に関係する変数もleftとcpの二個あるしね。
特定条件でcp戻さずleftだけ戻してたからこれで死んだ臭い。

cp使いたい気持ちはわかるし一々cpとbufの差を計算したくないのも分かるんだけどねぇ……
c++のイテレータとかからすると予めbufendを計算するべきなんだけど、
cではあんま見ないよねそういう書き方。

Re:

[Anonymous Coward]

言語問わずこんな感じのありますね。
無論燃えた後か継続中かの違いしか無い…

ムダに状態持つな
→フラグ持つな
→→bool使うな（）
なトコのコードもこんな感じだった…w
#一般常識と騙ってるのでたち悪かったな

Re:

[Anonymous Coward]

やっぱgotoって必要だよなーって思う。
どこからでも飛べてどこへでも飛べると複雑になりすぎるのでナウい言語(21世紀初頭くらい)の制限が必要だけども。

Re:

[Anonymous Coward]

使い手のスキル次第で猛毒になるだけですよねホント。。
そしてそんなのはコンパイラは弾けない…
#同一メソッド内コードクローンも弾けそうで弾けない…

Re:

[Anonymous Coward]

コピペコードは
sudo_term_killの所(便宜上A、脆弱性本体？)、
sudo_term_eraseの所(便宜上B)、
ループ後の所(便宜上C)、
で三ケ所あるけど、コピペ元はCっぽい気しない？

Cの箇所でだけ有効なエラー見てbreakをコピって生まれたバグや不整合に思える。
Aではポインタ戻し忘れてオーバーフロー、Bでは入力の中断が発生した。
ただBは他との比較ではおかしいけど、IOエラーで入力終了して既読分だけ処理ってままある処置かと。

writeエラー無視して処理続ける方針なら、全部writeエラー無視で良かった筈。

デクリメントの前置後置とか脆弱性とは無関係な修正も混ざってるから
パッチの意図がそもそもブレてんだよね。そのくせCはデクリメント変え忘れ。
コピペコードの一括修正で漏れとかデバッグ二次災害でありがちな光景だなと。