[ruby-trunk - Bug #6145][Open] two possible bugs in Onigmo

Issue #6145 has been reported by Yusuke E…


Bug #6145: two possible bugs in Onigmo
https://bugs.ruby-lang.org/issues/6145

Author: Yusuke E.
Status: Open
Priority: Normal
Assignee: Yui NARUSE
Category:
Target version:
ruby -v: ruby 2.0.0dev (2012-03-14 trunk 35017) [i686-linux]

naruse さん、k-takata さん
遠藤です。

Coverity Scan さんが見つけてくれたバグの可能性 2 つです。
(ただし false positive の可能性は高い)

1 つめ。regcomp.c の compile_length_enclose_node という関数で

1222 if (node->target) {
1223 tlen = compile_length_tree(node->target, reg);
1224 if (tlen < 0) return tlen;
1225 }
1226 else
1227 tlen = 0;

というように、node->target が NULL の可能性を考慮しているくせに、
その後で

1269 case ENCLOSE_CONDITION:
1270 len = SIZE_OP_CONDITION;
1271 if (NTYPE(node->target) == NT_ALT) {

と、node->target を遠慮なくデリファレンスしていて、まずいんじゃ
ないの?とのことです。

実際に ENCLOSE_CONDITION のケースで node->target が NULL になる
ことはないのかな?と思いましたが、確認お願いします。
安全な場合でも NULL check を入れてくれると、coverity scan さんの
alert が減って嬉しいです。(必須ではありません)

2 つめ。regparse.c に以下のようなコードがあります。

5974 *np = node_new_cclass();
5975 CHECK_NULL_RETURN_MEMERR(*np);
5976 cc = NCCLASS(*np);
5977 add_ctype_to_cc(cc, tok->u.prop.ctype, 0, 0, env);
5978 if (tok->u.prop.not != 0) NCCLASS_SET_NOT(cc);
5979 #ifdef USE_SHARED_CCLASS_TABLE
5980 }

5977 行目の add_ctype_to_cc の返り値をチェックしてません。
他の add_ctype_to_cc の呼び出しではことごとく返り値チェックして
いるので、ここだけ忘れてんじゃないの?いいの?とのことです。

実際に add_ctype_to_cc の返り値が 0 以外になるケースがあるのかは
わかりませんでしたが、まあチェックしてあげるといい気がします。


Yusuke E. [email protected]

Issue #6145 has been updated by k_takata (Ken Takata).

1つめは、ENCLOSE_CONDITION で node->target が NULL になることはありません。
チェック無しに node->target をデリファレンスをしているのは、ENCLOSE_STOP_BACKTRACK
のケースも同様なので、今回は修正していません。

2つめは修正しました。
https://github.com/k-takata/Onigmo/tree/tmp/ruby-2.0.x

Bug #6145: two possible bugs in Onigmo
https://bugs.ruby-lang.org/issues/6145#change-24976

Author: mame (Yusuke E.)
Status: Assigned
Priority: Normal
Assignee: naruse (Yui NARUSE)
Category:
Target version:
ruby -v: ruby 2.0.0dev (2012-03-14 trunk 35017) [i686-linux]

naruse さん、k-takata さん
遠藤です。

Coverity Scan さんが見つけてくれたバグの可能性 2 つです。
(ただし false positive の可能性は高い)

1 つめ。regcomp.c の compile_length_enclose_node という関数で

1222 if (node->target) {
1223 tlen = compile_length_tree(node->target, reg);
1224 if (tlen < 0) return tlen;
1225 }
1226 else
1227 tlen = 0;

というように、node->target が NULL の可能性を考慮しているくせに、
その後で

1269 case ENCLOSE_CONDITION:
1270 len = SIZE_OP_CONDITION;
1271 if (NTYPE(node->target) == NT_ALT) {

と、node->target を遠慮なくデリファレンスしていて、まずいんじゃ
ないの?とのことです。

実際に ENCLOSE_CONDITION のケースで node->target が NULL になる
ことはないのかな?と思いましたが、確認お願いします。
安全な場合でも NULL check を入れてくれると、coverity scan さんの
alert が減って嬉しいです。(必須ではありません)

2 つめ。regparse.c に以下のようなコードがあります。

5974 *np = node_new_cclass();
5975 CHECK_NULL_RETURN_MEMERR(*np);
5976 cc = NCCLASS(*np);
5977 add_ctype_to_cc(cc, tok->u.prop.ctype, 0, 0, env);
5978 if (tok->u.prop.not != 0) NCCLASS_SET_NOT(cc);
5979 #ifdef USE_SHARED_CCLASS_TABLE
5980 }

5977 行目の add_ctype_to_cc の返り値をチェックしてません。
他の add_ctype_to_cc の呼び出しではことごとく返り値チェックして
いるので、ここだけ忘れてんじゃないの?いいの?とのことです。

実際に add_ctype_to_cc の返り値が 0 以外になるケースがあるのかは
わかりませんでしたが、まあチェックしてあげるといい気がします。


Yusuke E. [email protected]

Issue #6145 has been updated by naruse (Yui NARUSE).

Status changed from Assigned to Closed


Bug #6145: two possible bugs in Onigmo
https://bugs.ruby-lang.org/issues/6145#change-25029

Author: mame (Yusuke E.)
Status: Closed
Priority: Normal
Assignee: naruse (Yui NARUSE)
Category:
Target version:
ruby -v: ruby 2.0.0dev (2012-03-14 trunk 35017) [i686-linux]

naruse さん、k-takata さん
遠藤です。

Coverity Scan さんが見つけてくれたバグの可能性 2 つです。
(ただし false positive の可能性は高い)

1 つめ。regcomp.c の compile_length_enclose_node という関数で

1222 if (node->target) {
1223 tlen = compile_length_tree(node->target, reg);
1224 if (tlen < 0) return tlen;
1225 }
1226 else
1227 tlen = 0;

というように、node->target が NULL の可能性を考慮しているくせに、
その後で

1269 case ENCLOSE_CONDITION:
1270 len = SIZE_OP_CONDITION;
1271 if (NTYPE(node->target) == NT_ALT) {

と、node->target を遠慮なくデリファレンスしていて、まずいんじゃ
ないの?とのことです。

実際に ENCLOSE_CONDITION のケースで node->target が NULL になる
ことはないのかな?と思いましたが、確認お願いします。
安全な場合でも NULL check を入れてくれると、coverity scan さんの
alert が減って嬉しいです。(必須ではありません)

2 つめ。regparse.c に以下のようなコードがあります。

5974 *np = node_new_cclass();
5975 CHECK_NULL_RETURN_MEMERR(*np);
5976 cc = NCCLASS(*np);
5977 add_ctype_to_cc(cc, tok->u.prop.ctype, 0, 0, env);
5978 if (tok->u.prop.not != 0) NCCLASS_SET_NOT(cc);
5979 #ifdef USE_SHARED_CCLASS_TABLE
5980 }

5977 行目の add_ctype_to_cc の返り値をチェックしてません。
他の add_ctype_to_cc の呼び出しではことごとく返り値チェックして
いるので、ここだけ忘れてんじゃないの?いいの?とのことです。

実際に add_ctype_to_cc の返り値が 0 以外になるケースがあるのかは
わかりませんでしたが、まあチェックしてあげるといい気がします。


Yusuke E. [email protected]

This forum is not affiliated to the Ruby language, Ruby on Rails framework, nor any Ruby applications discussed here.

| Privacy Policy | Terms of Service | Remote Ruby Jobs