From 83005d83d8ba95524436409d5d73fd82b63bc115 Mon Sep 17 00:00:00 2001 From: Craig Lehmann Date: Tue, 15 Nov 2016 14:50:52 -0500 Subject: Read length after args in String#setbyte Prevents RCE Reported by https://hackerone.com/raydot --- mrbgems/mruby-string-ext/test/string.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'mrbgems/mruby-string-ext/test/string.rb') diff --git a/mrbgems/mruby-string-ext/test/string.rb b/mrbgems/mruby-string-ext/test/string.rb index a5d55a7ee..228a236af 100644 --- a/mrbgems/mruby-string-ext/test/string.rb +++ b/mrbgems/mruby-string-ext/test/string.rb @@ -30,6 +30,18 @@ assert('String#setbyte') do assert_equal("Hello", str1) end +assert("String#setbyte raises IndexError if arg conversion resizes String") do + $s = "01234\n" + class Tmp + def to_i + $s.chomp! '' + 95 + end + end + tmp = Tmp.new + assert_raise(IndexError) { $s.setbyte(5, tmp) } +end + assert('String#byteslice') do str1 = "hello" assert_equal("e", str1.byteslice(1)) -- cgit v1.2.3 From 981105b3e6758455646e9834b1c2695bf774a401 Mon Sep 17 00:00:00 2001 From: Tomasz Dabrowski Date: Fri, 10 Feb 2017 12:58:41 +0100 Subject: String#ljust and String#rjust reimplementation (fix #3445) - String#ljust and String#rjust are now C functions to improve performance - infinite loop because of an empty padding argument is now prevented (ArgumentError is raised) - extra tests for ljust/rjust added --- mrbgems/mruby-string-ext/mrblib/string.rb | 46 ---------------- mrbgems/mruby-string-ext/src/string.c | 92 +++++++++++++++++++++++++++++++ mrbgems/mruby-string-ext/test/string.rb | 20 +++++++ 3 files changed, 112 insertions(+), 46 deletions(-) (limited to 'mrbgems/mruby-string-ext/test/string.rb') diff --git a/mrbgems/mruby-string-ext/mrblib/string.rb b/mrbgems/mruby-string-ext/mrblib/string.rb index 6e5f3c73d..610a462a7 100644 --- a/mrbgems/mruby-string-ext/mrblib/string.rb +++ b/mrbgems/mruby-string-ext/mrblib/string.rb @@ -263,52 +263,6 @@ class String self end - ## - # call-seq: - # str.ljust(integer, padstr=' ') -> new_str - # - # If integer is greater than the length of str, returns a new - # String of length integer with str left justified - # and padded with padstr; otherwise, returns str. - # - # "hello".ljust(4) #=> "hello" - # "hello".ljust(20) #=> "hello " - # "hello".ljust(20, '1234') #=> "hello123412341234123" - def ljust(idx, padstr = ' ') - if idx <= self.size - return self - end - newstr = self.dup - newstr << padstr - while newstr.size <= idx - newstr << padstr - end - return newstr.slice(0,idx) - end - - ## - # call-seq: - # str.rjust(integer, padstr=' ') -> new_str - # - # If integer is greater than the length of str, returns a new - # String of length integer with str right justified - # and padded with padstr; otherwise, returns str. - # - # "hello".rjust(4) #=> "hello" - # "hello".rjust(20) #=> " hello" - # "hello".rjust(20, '1234') #=> "123412341234123hello" - def rjust(idx, padstr = ' ') - if idx <= self.size - return self - end - padsize = idx - self.size - newstr = padstr.dup - while newstr.size <= padsize - newstr << padstr - end - return newstr.slice(0,padsize) + self - end - # str.upto(other_str, exclusive=false) {|s| block } -> str # str.upto(other_str, exclusive=false) -> an_enumerator # diff --git a/mrbgems/mruby-string-ext/src/string.c b/mrbgems/mruby-string-ext/src/string.c index 7e87b3db4..45d406f6f 100644 --- a/mrbgems/mruby-string-ext/src/string.c +++ b/mrbgems/mruby-string-ext/src/string.c @@ -5,6 +5,9 @@ #include #include +#define MAX(a,b) ((a)>(b) ? (a) : (b)) +#define MIN(a,b) ((a)<(b) ? (a) : (b)) + static mrb_value mrb_str_getbyte(mrb_state *mrb, mrb_value str) { @@ -509,6 +512,93 @@ mrb_str_ord(mrb_state* mrb, mrb_value str) } #endif +static mrb_value +mrb_str_just(mrb_state* mrb, mrb_value str, mrb_bool right_just) +{ + mrb_value new_str; + mrb_int idx = 0, i = 0, bytes_to_copy = 0, start_pos = 0, final_pos = 0, + pad_str_length = 0; + mrb_int str_length = RSTRING_LEN(str); + const char *pad_str = NULL; + char *new_str_ptr = NULL; + + mrb_get_args(mrb, "i|s!", &idx, &pad_str, &pad_str_length); + + if (pad_str == NULL) + { + pad_str = " "; + pad_str_length = 1; + } + + if (pad_str_length == 0) + { + mrb_raise(mrb, E_ARGUMENT_ERROR, "zero width padding"); + } + + if (idx <= str_length) + { + return str; + } + + new_str = mrb_str_dup(mrb, str); + mrb_str_resize(mrb, new_str, idx); + + new_str_ptr = RSTRING_PTR(new_str); + + if (right_just) + { + memcpy(new_str_ptr + idx - str_length, RSTRING_PTR(str), str_length); + } + + start_pos = right_just ? 0 : str_length; + final_pos = idx - (right_just ? str_length : 0); + + for (i = start_pos; i < final_pos; i += pad_str_length) + { + bytes_to_copy = idx - i - (right_just ? str_length : 0); + bytes_to_copy = MIN(pad_str_length, bytes_to_copy); + memcpy(new_str_ptr + i, pad_str, bytes_to_copy); + } + + return new_str; +} + +/* + * call-seq: + * str.ljust(integer, padstr=' ') -> new_str + * + * If integer is greater than the length of str, returns a new + * String of length integer with str left justified + * and padded with padstr; otherwise, returns str. + * + * "hello".ljust(4) #=> "hello" + * "hello".ljust(20) #=> "hello " + * "hello".ljust(20, '1234') #=> "hello123412341234123" + */ +static mrb_value +mrb_str_ljust(mrb_state* mrb, mrb_value str) +{ + return mrb_str_just(mrb, str, FALSE); +} + +/* + * call-seq: + * str.rjust(integer, padstr=' ') -> new_str + * + * If integer is greater than the length of str, returns a new + * String of length integer with str right justified + * and padded with padstr; otherwise, returns str. + * + * "hello".rjust(4) #=> "hello" + * "hello".rjust(20) #=> " hello" + * "hello".rjust(20, '1234') #=> "123412341234123hello" + */ +static mrb_value +mrb_str_rjust(mrb_state* mrb, mrb_value str) +{ + return mrb_str_just(mrb, str, TRUE); +} + void mrb_mruby_string_ext_gem_init(mrb_state* mrb) { @@ -530,6 +620,8 @@ mrb_mruby_string_ext_gem_init(mrb_state* mrb) mrb_define_method(mrb, s, "lines", mrb_str_lines, MRB_ARGS_NONE()); mrb_define_method(mrb, s, "succ", mrb_str_succ, MRB_ARGS_NONE()); mrb_define_method(mrb, s, "succ!", mrb_str_succ_bang, MRB_ARGS_NONE()); + mrb_define_method(mrb, s, "ljust", mrb_str_ljust, MRB_ARGS_REQ(1)|MRB_ARGS_OPT(1)); + mrb_define_method(mrb, s, "rjust", mrb_str_rjust, MRB_ARGS_REQ(1)|MRB_ARGS_OPT(1)); mrb_alias_method(mrb, s, mrb_intern_lit(mrb, "next"), mrb_intern_lit(mrb, "succ")); mrb_alias_method(mrb, s, mrb_intern_lit(mrb, "next!"), mrb_intern_lit(mrb, "succ!")); mrb_define_method(mrb, s, "ord", mrb_str_ord, MRB_ARGS_NONE()); diff --git a/mrbgems/mruby-string-ext/test/string.rb b/mrbgems/mruby-string-ext/test/string.rb index 228a236af..2b2c02b8b 100644 --- a/mrbgems/mruby-string-ext/test/string.rb +++ b/mrbgems/mruby-string-ext/test/string.rb @@ -444,6 +444,26 @@ assert('String#rjust') do assert_equal "hello", "hello".rjust(-3) end +assert('String#ljust should not change string') do + a = "hello" + a.ljust(20) + assert_equal "hello", a +end + +assert('String#rjust should not change string') do + a = "hello" + a.rjust(20) + assert_equal "hello", a +end + +assert('String#ljust should raise on zero width padding') do + assert_raise(ArgumentError) { "foo".ljust(10, '') } +end + +assert('String#rjust should raise on zero width padding') do + assert_raise(ArgumentError) { "foo".rjust(10, '') } +end + assert('String#upto') do a = "aa" start = "aa" -- cgit v1.2.3 From 24048cd998777757147b14948e0a149dffcac76d Mon Sep 17 00:00:00 2001 From: Tomasz Dabrowski Date: Fri, 10 Feb 2017 14:44:36 +0100 Subject: Tests for UTF-8 String#ljust and String#rjust --- mrbgems/mruby-string-ext/test/string.rb | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) (limited to 'mrbgems/mruby-string-ext/test/string.rb') diff --git a/mrbgems/mruby-string-ext/test/string.rb b/mrbgems/mruby-string-ext/test/string.rb index 2b2c02b8b..996ad2669 100644 --- a/mrbgems/mruby-string-ext/test/string.rb +++ b/mrbgems/mruby-string-ext/test/string.rb @@ -433,6 +433,7 @@ end assert('String#ljust') do assert_equal "hello", "hello".ljust(4) assert_equal "hello ", "hello".ljust(20) + assert_equal 20, "hello".ljust(20).length assert_equal "hello123412341234123", "hello".ljust(20, '1234') assert_equal "hello", "hello".ljust(-3) end @@ -440,10 +441,39 @@ end assert('String#rjust') do assert_equal "hello", "hello".rjust(4) assert_equal " hello", "hello".rjust(20) + assert_equal 20, "hello".rjust(20).length assert_equal "123412341234123hello", "hello".rjust(20, '1234') assert_equal "hello", "hello".rjust(-3) end +if UTF8STRING + assert('String#ljust with UTF8') do + assert_equal "helloん ", "helloん".ljust(20) + assert_equal "helloó ", "helloó".ljust(34) + assert_equal 34, "helloó".ljust(34).length + assert_equal "helloんんんんんんんんんんんんんん", "hello".ljust(19, 'ん') + assert_equal "helloんんんんんんんんんんんんんんん", "hello".ljust(20, 'ん') + end + + assert('String#rjust with UTF8') do + assert_equal " helloん", "helloん".rjust(20) + assert_equal " helloó", "helloó".rjust(34) + # assert_equal 34, "helloó".rjust(34).length + assert_equal "んんんんんんんんんんんんんんhello", "hello".rjust(19, 'ん') + assert_equal "んんんんんんんんんんんんんんんhello", "hello".rjust(20, 'ん') + end + + assert('UTF8 byte counting') do + skip('string length is broken after []=') + + # based on assert_equal 34, "helloó".rjust(34).length + ret = ' ' + ret[-6..-1] = "helloó" + + assert_equal 34, ret.length + end +end + assert('String#ljust should not change string') do a = "hello" a.ljust(20) -- cgit v1.2.3 From b130f43298b03f2b1e8e1bf86c236a6f0caadfb3 Mon Sep 17 00:00:00 2001 From: robert Date: Sat, 11 Feb 2017 07:40:55 +0000 Subject: remove skip that shouldn't be necessary anymore. the test should pass after https://github.com/mruby/mruby/commit/8f4a929e1a01c8d6176fb53a9ef5dff6de632959. --- mrbgems/mruby-string-ext/test/string.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'mrbgems/mruby-string-ext/test/string.rb') diff --git a/mrbgems/mruby-string-ext/test/string.rb b/mrbgems/mruby-string-ext/test/string.rb index 996ad2669..9b7e99647 100644 --- a/mrbgems/mruby-string-ext/test/string.rb +++ b/mrbgems/mruby-string-ext/test/string.rb @@ -464,12 +464,10 @@ if UTF8STRING end assert('UTF8 byte counting') do - skip('string length is broken after []=') - # based on assert_equal 34, "helloó".rjust(34).length + # see https://github.com/mruby/mruby/issues/3448 ret = ' ' ret[-6..-1] = "helloó" - assert_equal 34, ret.length end end -- cgit v1.2.3 From 0b143898c4203ce611dc0d270569fc576f894758 Mon Sep 17 00:00:00 2001 From: "Yukihiro \"Matz\" Matsumoto" Date: Sat, 11 Feb 2017 16:52:45 +0900 Subject: Remove historical comment; ref #3450 #3448 --- mrbgems/mruby-string-ext/test/string.rb | 2 -- 1 file changed, 2 deletions(-) (limited to 'mrbgems/mruby-string-ext/test/string.rb') diff --git a/mrbgems/mruby-string-ext/test/string.rb b/mrbgems/mruby-string-ext/test/string.rb index 9b7e99647..24bc859d8 100644 --- a/mrbgems/mruby-string-ext/test/string.rb +++ b/mrbgems/mruby-string-ext/test/string.rb @@ -464,8 +464,6 @@ if UTF8STRING end assert('UTF8 byte counting') do - # based on assert_equal 34, "helloó".rjust(34).length - # see https://github.com/mruby/mruby/issues/3448 ret = ' ' ret[-6..-1] = "helloó" assert_equal 34, ret.length -- cgit v1.2.3