Hello, On Wed, Nov 16, 2016 at 02:48:03AM +0100, Christian Hofstaedtler wrote: > Hi, > > * Salvatore Bonaccorso <car...@debian.org> [161116 01:46]: > > Source: ruby2.3 > > [...] > > [0] https://security-tracker.debian.org/tracker/CVE-2016-7798 > > [1] https://github.com/ruby/openssl/issues/49 > > [2] > > https://github.com/ruby/openssl/commit/8108e0a6db133f3375608303fdd2083eb5115062 > > I'm attaching a potential patch against ruby2.3 2.3.2. Any review > would be most welcome.
On Fri, Jun 16, 2017 at 09:17:37AM +0200, Salvatore Bonaccorso wrote: > Source: ruby2.3 > Version: 2.3.3-1 > Severity: important > Tags: upstream security patch > > Hi, > > the following vulnerability was published for ruby2.3. > > CVE-2015-9096[0]: > | Net::SMTP in Ruby before 2.4.0 is vulnerable to SMTP command injection > | via CRLF sequences in a RCPT TO or MAIL FROM command, as demonstrated > | by CRLF sequences immediately before and after a DATA substring. > > If you fix the vulnerability please also make sure to include the > CVE (Common Vulnerabilities & Exposures) id in your changelog entry. > > For further information see: > > [0] https://security-tracker.debian.org/tracker/CVE-2015-9096 > https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2015-9096 > [1] > https://github.com/ruby/ruby/commit/0827a7e52ba3d957a634b063bf5a391239b9ffee On Thu, Aug 31, 2017 at 12:15:00PM +0200, Raphael Hertzog wrote: > Source: ruby2.3 > X-Debbugs-CC: t...@security.debian.org > secure-testing-t...@lists.alioth.debian.org > Severity: important > Tags: security > > Hi, > > the following vulnerabilities were published for ruby2.3. They affect rubygems > more specifically. > > CVE-2017-0902[0]: > DNS issue > > CVE-2017-0901[1]: > overwrite any file > > CVE-2017-0900[2]: > query command > > CVE-2017-0899[3]: > ANSI escape issue > > Some patches are available here: > https://www.ruby-lang.org/en/news/2017/08/29/multiple-vulnerabilities-in-rubygems/ > > The fixes should also be available in (upcoming) ruby 2.3.5 and ruby 2.4.2. > > If you fix the vulnerabilities please also make sure to include the > CVE (Common Vulnerabilities & Exposures) ids in your changelog entry. > > For further information see: > > [0] https://security-tracker.debian.org/tracker/CVE-2017-0902 > https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-0902 > [1] https://security-tracker.debian.org/tracker/CVE-2017-0901 > https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-0901 > [2] https://security-tracker.debian.org/tracker/CVE-2017-0900 > https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-0900 > [3] https://security-tracker.debian.org/tracker/CVE-2017-0899 > https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-0899 > > Please adjust the affected versions in the BTS as needed. On Fri, Sep 01, 2017 at 07:24:24AM +0200, Salvatore Bonaccorso wrote: > Source: ruby2.3 > Version: 2.3.3-1 > Severity: grave > Tags: upstream patch security > > Hi, > > the following vulnerability was published for ruby2.3. > > CVE-2017-14064[0]: > | Ruby through 2.2.7, 2.3.x through 2.3.4, and 2.4.x through 2.4.1 can > | expose arbitrary memory during a JSON.generate call. The issues lies in > | using strdup in ext/json/ext/generator/generator.c, which will stop > | after encountering a '\0' byte, returning a pointer to a string of > | length zero, which is not the length stored in space_len. > > If you fix the vulnerability please also make sure to include the > CVE (Common Vulnerabilities & Exposures) id in your changelog entry. > > For further information see: > > [0] https://security-tracker.debian.org/tracker/CVE-2017-14064 > https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-14064 > [1] https://bugs.ruby-lang.org/issues/13853 > [2] > https://github.com/flori/json/commit/8f782fd8e181d9cfe9387ded43a5ca9692266b85 I have prepared an updated ruby2.3 package that handles all of the above issues, which are all pending security bugs in the BTS. Package builds fine and autopkgtest passes, what is good because all of the patches applied include automated tests for the issues they fix. Attached you will find a full diff between the version in stretch and the one I intend to upload, plus the individual patches for your convenience. Security team: I'm waiting on your ACK. Fixing this in unstable/buster will take longer, because the current version there does not build with GCC 7. Take into account that ruby2.3 won't be shipped in buster anyway, it will only stay there until we transition to ruby2.5.
diff --git a/debian/changelog b/debian/changelog index 01d9695b..068662c7 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,32 @@ +ruby2.3 (2.3.3-1+deb9u1) stretch-security; urgency=high + + * Fix arbitrary heap exposure problem in the JSON library (Closes: #873906) + [CVE-2017-14064] + - Backported for Ruby 2.3 by Hiroshi SHIBATA <h...@ruby-lang.org> + https://bugs.ruby-lang.org/issues/13853 + * Fix multiple security vulnerabilities in Rubygems (Closes: #873802) + - Fix a DNS request hijacking vulnerability. Discovered by Jonathan + Claudius, fix by Samuel Giddins. + [CVE-2017-0902] + - Fix an ANSI escape sequence vulnerability. Discovered by Yusuke Endoh, + fix by Evan Phoenix. + [CVE-2017-0899] + - Fix a DOS vulernerability in the query command. Discovered by Yusuke + Endoh, fix by Samuel Giddins. + [CVE-2017-0900] + - Fix a vulnerability in the gem installer that allowed a malicious gem to + overwrite arbitrary files. Discovered by Yusuke Endoh, fix by Samuel + Giddins. + [CVE-2017-0901] + * Fix SMTP comment injection (Closes: #864860) + Patch by Shugo Maeda <sh...@ruby-lang.org> + [CVE-2015-9096] + * Fix IV Reuse in GCM Mode (Closes: #842432) + Patch by Kazuki Yamaguchi <k...@rhe.jp> + [CVE-2016-7798] + + -- Antonio Terceiro <terce...@debian.org> Sat, 02 Sep 2017 15:11:07 -0300 + ruby2.3 (2.3.3-1) unstable; urgency=medium * New upstream version. diff --git a/ext/json/generator/generator.c b/ext/json/generator/generator.c index a135e283..2cdca568 100644 --- a/ext/json/generator/generator.c +++ b/ext/json/generator/generator.c @@ -301,7 +301,7 @@ static char *fstrndup(const char *ptr, unsigned long len) { char *result; if (len <= 0) return NULL; result = ALLOC_N(char, len); - memccpy(result, ptr, 0, len); + memcpy(result, ptr, len); return result; } @@ -1055,7 +1055,7 @@ static VALUE cState_indent_set(VALUE self, VALUE indent) } } else { if (state->indent) ruby_xfree(state->indent); - state->indent = strdup(RSTRING_PTR(indent)); + state->indent = fstrndup(RSTRING_PTR(indent), len); state->indent_len = len; } return Qnil; @@ -1093,7 +1093,7 @@ static VALUE cState_space_set(VALUE self, VALUE space) } } else { if (state->space) ruby_xfree(state->space); - state->space = strdup(RSTRING_PTR(space)); + state->space = fstrndup(RSTRING_PTR(space), len); state->space_len = len; } return Qnil; @@ -1129,7 +1129,7 @@ static VALUE cState_space_before_set(VALUE self, VALUE space_before) } } else { if (state->space_before) ruby_xfree(state->space_before); - state->space_before = strdup(RSTRING_PTR(space_before)); + state->space_before = fstrndup(RSTRING_PTR(space_before), len); state->space_before_len = len; } return Qnil; @@ -1166,7 +1166,7 @@ static VALUE cState_object_nl_set(VALUE self, VALUE object_nl) } } else { if (state->object_nl) ruby_xfree(state->object_nl); - state->object_nl = strdup(RSTRING_PTR(object_nl)); + state->object_nl = fstrndup(RSTRING_PTR(object_nl), len); state->object_nl_len = len; } return Qnil; @@ -1201,7 +1201,7 @@ static VALUE cState_array_nl_set(VALUE self, VALUE array_nl) } } else { if (state->array_nl) ruby_xfree(state->array_nl); - state->array_nl = strdup(RSTRING_PTR(array_nl)); + state->array_nl = fstrndup(RSTRING_PTR(array_nl), len); state->array_nl_len = len; } return Qnil; diff --git a/ext/json/generator/generator.h b/ext/json/generator/generator.h index 298c0a49..6bbf817b 100644 --- a/ext/json/generator/generator.h +++ b/ext/json/generator/generator.h @@ -1,7 +1,6 @@ #ifndef _GENERATOR_H_ #define _GENERATOR_H_ -#include <string.h> #include <math.h> #include <ctype.h> diff --git a/ext/json/lib/json/version.rb b/ext/json/lib/json/version.rb index b5748334..cd7ddf87 100644 --- a/ext/json/lib/json/version.rb +++ b/ext/json/lib/json/version.rb @@ -1,7 +1,7 @@ # frozen_string_literal: false module JSON # JSON version - VERSION = '1.8.3' + VERSION = '1.8.3.1' VERSION_ARRAY = VERSION.split(/\./).map { |x| x.to_i } # :nodoc: VERSION_MAJOR = VERSION_ARRAY[0] # :nodoc: VERSION_MINOR = VERSION_ARRAY[1] # :nodoc: diff --git a/ext/openssl/ossl_cipher.c b/ext/openssl/ossl_cipher.c index 09b021d9..24b84679 100644 --- a/ext/openssl/ossl_cipher.c +++ b/ext/openssl/ossl_cipher.c @@ -34,6 +34,7 @@ */ VALUE cCipher; VALUE eCipherError; +static ID id_key_set; static VALUE ossl_cipher_alloc(VALUE klass); static void ossl_cipher_free(void *ptr); @@ -114,7 +115,6 @@ ossl_cipher_initialize(VALUE self, VALUE str) EVP_CIPHER_CTX *ctx; const EVP_CIPHER *cipher; char *name; - unsigned char key[EVP_MAX_KEY_LENGTH]; name = StringValuePtr(str); GetCipherInit(self, ctx); @@ -126,14 +126,7 @@ ossl_cipher_initialize(VALUE self, VALUE str) if (!(cipher = EVP_get_cipherbyname(name))) { ossl_raise(rb_eRuntimeError, "unsupported cipher algorithm (%s)", name); } - /* - * The EVP which has EVP_CIPH_RAND_KEY flag (such as DES3) allows - * uninitialized key, but other EVPs (such as AES) does not allow it. - * Calling EVP_CipherUpdate() without initializing key causes SEGV so we - * set the data filled with "\0" as the key by default. - */ - memset(key, 0, EVP_MAX_KEY_LENGTH); - if (EVP_CipherInit_ex(ctx, cipher, NULL, key, NULL, -1) != 1) + if (EVP_CipherInit_ex(ctx, cipher, NULL, NULL, NULL, -1) != 1) ossl_raise(eCipherError, NULL); return self; @@ -252,6 +245,9 @@ ossl_cipher_init(int argc, VALUE *argv, VALUE self, int mode) ossl_raise(eCipherError, NULL); } + if (p_key) + rb_ivar_set(self, id_key_set, Qtrue); + return self; } @@ -338,6 +334,8 @@ ossl_cipher_pkcs5_keyivgen(int argc, VALUE *argv, VALUE self) OPENSSL_cleanse(key, sizeof key); OPENSSL_cleanse(iv, sizeof iv); + rb_ivar_set(self, id_key_set, Qtrue); + return Qnil; } @@ -390,6 +388,8 @@ ossl_cipher_update(int argc, VALUE *argv, VALUE self) VALUE data, str; rb_scan_args(argc, argv, "11", &data, &str); + if (!RTEST(rb_attr_get(self, id_key_set))) + ossl_raise(eCipherError, "key not set"); StringValue(data); in = (unsigned char *)RSTRING_PTR(data); @@ -490,6 +490,8 @@ ossl_cipher_set_key(VALUE self, VALUE key) if (EVP_CipherInit_ex(ctx, NULL, NULL, (unsigned char *)RSTRING_PTR(key), NULL, -1) != 1) ossl_raise(eCipherError, NULL); + rb_ivar_set(self, id_key_set, Qtrue); + return key; } @@ -1008,4 +1010,6 @@ Init_ossl_cipher(void) rb_define_method(cCipher, "iv_len", ossl_cipher_iv_length, 0); rb_define_method(cCipher, "block_size", ossl_cipher_block_size, 0); rb_define_method(cCipher, "padding=", ossl_cipher_set_padding, 1); + + id_key_set = rb_intern_const("key_set"); } diff --git a/lib/net/smtp.rb b/lib/net/smtp.rb index d634274c..78f2181d 100644 --- a/lib/net/smtp.rb +++ b/lib/net/smtp.rb @@ -926,7 +926,15 @@ def quit private + def validate_line(line) + # A bare CR or LF is not allowed in RFC5321. + if /[\r\n]/ =~ line + raise ArgumentError, "A line must not contain CR or LF" + end + end + def getok(reqline) + validate_line reqline res = critical { @socket.writeline reqline recv_response() @@ -936,6 +944,7 @@ def getok(reqline) end def get_response(reqline) + validate_line reqline @socket.writeline reqline recv_response() end diff --git a/lib/rubygems.rb b/lib/rubygems.rb index 04031c76..9c0219ce 100644 --- a/lib/rubygems.rb +++ b/lib/rubygems.rb @@ -10,7 +10,7 @@ require 'thread' module Gem - VERSION = '2.5.2' + VERSION = '2.5.2.1' end # Must be first since it unloads the prelude from 1.9.2 diff --git a/lib/rubygems/commands/query_command.rb b/lib/rubygems/commands/query_command.rb index d6196b44..61e98088 100644 --- a/lib/rubygems/commands/query_command.rb +++ b/lib/rubygems/commands/query_command.rb @@ -226,7 +226,7 @@ def output_versions output, versions end end - output << make_entry(matching_tuples, platforms) + output << clean_text(make_entry(matching_tuples, platforms)) end end @@ -344,7 +344,8 @@ def spec_platforms entry, platforms end def spec_summary entry, spec - entry << "\n\n" << format_text(spec.summary, 68, 4) + summary = truncate_text(spec.summary, "the summary for #{spec.full_name}") + entry << "\n\n" << format_text(summary, 68, 4) end end diff --git a/lib/rubygems/installer.rb b/lib/rubygems/installer.rb index 85358e0d..709b77d1 100644 --- a/lib/rubygems/installer.rb +++ b/lib/rubygems/installer.rb @@ -693,6 +693,11 @@ def verify_gem_home(unpack = false) # :nodoc: unpack or File.writable?(gem_home) end + def verify_spec_name + return if spec.name =~ Gem::Specification::VALID_NAME_PATTERN + raise Gem::InstallError, "#{spec} has an invalid name" + end + ## # Return the text for an application file. @@ -812,6 +817,8 @@ def pre_install_checks ensure_loadable_spec + verify_spec_name + if options[:install_as_default] Gem.ensure_default_gem_subdirectories gem_home else diff --git a/lib/rubygems/remote_fetcher.rb b/lib/rubygems/remote_fetcher.rb index fda1e067..254bebfa 100644 --- a/lib/rubygems/remote_fetcher.rb +++ b/lib/rubygems/remote_fetcher.rb @@ -104,7 +104,7 @@ def api_endpoint(uri) else target = res.target.to_s.strip - if /\.#{Regexp.quote(host)}\z/ =~ target + if URI("http://"; + target).host.end_with?(".#{host}") return URI.parse "#{uri.scheme}://#{target}#{uri.path}" end diff --git a/lib/rubygems/specification.rb b/lib/rubygems/specification.rb index 9f635b1f..2519b96b 100644 --- a/lib/rubygems/specification.rb +++ b/lib/rubygems/specification.rb @@ -108,6 +108,8 @@ class Gem::Specification < Gem::BasicSpecification private_constant :LOAD_CACHE if defined? private_constant + VALID_NAME_PATTERN = /\A[a-zA-Z0-9\.\-\_]+\z/ # :nodoc: + # :startdoc: ## @@ -2667,9 +2669,15 @@ def validate packaging = true end end - unless String === name then + if !name.is_a?(String) then + raise Gem::InvalidSpecificationException, + "invalid value for attribute name: \"#{name.inspect}\" must be a string" + elsif name !~ /[a-zA-Z]/ then + raise Gem::InvalidSpecificationException, + "invalid value for attribute name: #{name.dump} must include at least one letter" + elsif name !~ VALID_NAME_PATTERN then raise Gem::InvalidSpecificationException, - "invalid value for attribute name: \"#{name.inspect}\"" + "invalid value for attribute name: #{name.dump} can only include letters, numbers, dashes, and underscores" end if raw_require_paths.empty? then diff --git a/lib/rubygems/text.rb b/lib/rubygems/text.rb index 732f1b99..b944b62c 100644 --- a/lib/rubygems/text.rb +++ b/lib/rubygems/text.rb @@ -6,13 +6,26 @@ module Gem::Text + ## + # Remove any non-printable characters and make the text suitable for + # printing. + def clean_text(text) + text.gsub(/[\000-\b\v-\f\016-\037\177]/, ".".freeze) + end + + def truncate_text(text, description, max_length = 100_000) + raise ArgumentError, "max_length must be positive" unless max_length > 0 + return text if text.size <= max_length + "Truncating #{description} to #{max_length.to_s.reverse.gsub(/...(?=.)/,'\&,').reverse} characters:\n" + text[0, max_length] + end + ## # Wraps +text+ to +wrap+ characters and optionally indents by +indent+ # characters def format_text(text, wrap, indent=0) result = [] - work = text.dup + work = clean_text(text) while work.length > wrap do if work =~ /^(.{0,#{wrap}})[ \n]/ then diff --git a/test/net/smtp/test_smtp.rb b/test/net/smtp/test_smtp.rb index 0edb3419..3bcceb6f 100644 --- a/test/net/smtp/test_smtp.rb +++ b/test/net/smtp/test_smtp.rb @@ -6,6 +6,8 @@ module Net class TestSMTP < Test::Unit::TestCase class FakeSocket + attr_reader :write_io + def initialize out = "250 OK\n" @write_io = StringIO.new @read_io = StringIO.new out @@ -51,5 +53,50 @@ def test_rset assert smtp.rset end + + def test_mailfrom + sock = FakeSocket.new + smtp = Net::SMTP.new 'localhost', 25 + smtp.instance_variable_set :@socket, sock + assert smtp.mailfrom("f...@example.com").success? + assert_equal "MAIL FROM:<f...@example.com>\r\n", sock.write_io.string + end + + def test_rcptto + sock = FakeSocket.new + smtp = Net::SMTP.new 'localhost', 25 + smtp.instance_variable_set :@socket, sock + assert smtp.rcptto("f...@example.com").success? + assert_equal "RCPT TO:<f...@example.com>\r\n", sock.write_io.string + end + + def test_auth_plain + sock = FakeSocket.new + smtp = Net::SMTP.new 'localhost', 25 + smtp.instance_variable_set :@socket, sock + assert smtp.auth_plain("foo", "bar").success? + assert_equal "AUTH PLAIN AGZvbwBiYXI=\r\n", sock.write_io.string + end + + def test_crlf_injection + smtp = Net::SMTP.new 'localhost', 25 + smtp.instance_variable_set :@socket, FakeSocket.new + + assert_raise(ArgumentError) do + smtp.mailfrom("foo\r\nbar") + end + + assert_raise(ArgumentError) do + smtp.mailfrom("foo\rbar") + end + + assert_raise(ArgumentError) do + smtp.mailfrom("foo\nbar") + end + + assert_raise(ArgumentError) do + smtp.rcptto("foo\r\nbar") + end + end end end diff --git a/test/openssl/test_cipher.rb b/test/openssl/test_cipher.rb index 89c176f4..fb08b610 100644 --- a/test/openssl/test_cipher.rb +++ b/test/openssl/test_cipher.rb @@ -81,6 +81,7 @@ def test_reset def test_empty_data @c1.encrypt + @c1.random_key assert_raise(ArgumentError){ @c1.update("") } end @@ -129,12 +130,10 @@ def test_AES } end - def test_AES_crush - 500.times do - assert_nothing_raised("[Bug #2768]") do - # it caused OpenSSL SEGV by uninitialized key - OpenSSL::Cipher::AES128.new("ECB").update "." * 17 - end + def test_update_raise_if_key_not_set + assert_raise(OpenSSL::Cipher::CipherError) do + # it caused OpenSSL SEGV by uninitialized key + OpenSSL::Cipher::AES128.new("ECB").update "." * 17 end end end @@ -238,6 +237,24 @@ def test_aes_gcm_wrong_ciphertext end + def test_aes_gcm_key_iv_order_issue + pt = "[ruby/openssl#49]" + cipher = OpenSSL::Cipher.new("aes-128-gcm").encrypt + cipher.key = "x" * 16 + cipher.iv = "a" * 12 + ct1 = cipher.update(pt) << cipher.final + tag1 = cipher.auth_tag + + cipher = OpenSSL::Cipher.new("aes-128-gcm").encrypt + cipher.iv = "a" * 12 + cipher.key = "x" * 16 + ct2 = cipher.update(pt) << cipher.final + tag2 = cipher.auth_tag + + assert_equal ct1, ct2 + assert_equal tag1, tag2 + end if has_cipher?("aes-128-gcm") + private def new_encryptor(algo) diff --git a/test/rubygems/test_gem_commands_query_command.rb b/test/rubygems/test_gem_commands_query_command.rb index 78c15a17..9ec71549 100644 --- a/test/rubygems/test_gem_commands_query_command.rb +++ b/test/rubygems/test_gem_commands_query_command.rb @@ -116,6 +116,86 @@ def test_execute_details This is a lot of text. This is a lot of text. This is a lot of text. This is a lot of text. +pl (1) + Platform: i386-linux + Author: A User + Homepage: http://example.com + + this is a summary + EOF + + assert_equal expected, @ui.output + assert_equal '', @ui.error + end + + def test_execute_details_cleans_text + spec_fetcher do |fetcher| + fetcher.spec 'a', 2 do |s| + s.summary = 'This is a lot of text. ' * 4 + s.authors = ["Abraham Lincoln \x01", "\x02 Hirohito"] + s.homepage = "http://a.example.com/\x03"; + end + + fetcher.legacy_platform + end + + @cmd.handle_options %w[-r -d] + + use_ui @ui do + @cmd.execute + end + + expected = <<-EOF + +*** REMOTE GEMS *** + +a (2) + Authors: Abraham Lincoln ., . Hirohito + Homepage: http://a.example.com/. + + This is a lot of text. This is a lot of text. This is a lot of text. + This is a lot of text. + +pl (1) + Platform: i386-linux + Author: A User + Homepage: http://example.com + + this is a summary + EOF + + assert_equal expected, @ui.output + assert_equal '', @ui.error + end + + def test_execute_details_truncates_summary + spec_fetcher do |fetcher| + fetcher.spec 'a', 2 do |s| + s.summary = 'This is a lot of text. ' * 10_000 + s.authors = ["Abraham Lincoln \x01", "\x02 Hirohito"] + s.homepage = "http://a.example.com/\x03"; + end + + fetcher.legacy_platform + end + + @cmd.handle_options %w[-r -d] + + use_ui @ui do + @cmd.execute + end + + expected = <<-EOF + +*** REMOTE GEMS *** + +a (2) + Authors: Abraham Lincoln ., . Hirohito + Homepage: http://a.example.com/. + + Truncating the summary for a-2 to 100,000 characters: +#{" This is a lot of text. This is a lot of text. This is a lot of text.\n" * 1449} This is a lot of te + pl (1) Platform: i386-linux Author: A User diff --git a/test/rubygems/test_gem_installer.rb b/test/rubygems/test_gem_installer.rb index 5ec71d0a..1092a0c6 100644 --- a/test/rubygems/test_gem_installer.rb +++ b/test/rubygems/test_gem_installer.rb @@ -1227,6 +1227,26 @@ def test_pre_install_checks_wrong_rubygems_version end end + def test_pre_install_checks_malicious_name + spec = util_spec '../malicious', '1' + def spec.full_name # so the spec is buildable + "malicious-1" + end + def spec.validate; end + + util_build_gem spec + + gem = File.join(@gemhome, 'cache', spec.file_name) + + use_ui @ui do + @installer = Gem::Installer.at gem + e = assert_raises Gem::InstallError do + @installer.pre_install_checks + end + assert_equal '#<Gem::Specification name=../malicious version=1> has an invalid name', e.message + end + end + def test_shebang util_make_exec @spec, "#!/usr/bin/ruby" diff --git a/test/rubygems/test_gem_remote_fetcher.rb b/test/rubygems/test_gem_remote_fetcher.rb index 49b6b665..a3919c8e 100644 --- a/test/rubygems/test_gem_remote_fetcher.rb +++ b/test/rubygems/test_gem_remote_fetcher.rb @@ -253,6 +253,21 @@ def test_api_endpoint_ignores_trans_domain_values_that_end_with_original dns.verify end + def test_api_endpoint_ignores_trans_domain_values_that_end_with_original_in_path + uri = URI.parse "http://example.com/foo"; + target = MiniTest::Mock.new + target.expect :target, "evil.com/a.example.com" + + dns = MiniTest::Mock.new + dns.expect :getresource, target, [String, Object] + + fetch = Gem::RemoteFetcher.new nil, dns + assert_equal URI.parse("http://example.com/foo";), fetch.api_endpoint(uri) + + target.verify + dns.verify + end + def test_api_endpoint_timeout_warning uri = URI.parse "http://gems.example.com/foo"; diff --git a/test/rubygems/test_gem_specification.rb b/test/rubygems/test_gem_specification.rb index bc1c8d2c..9a49bbbf 100644 --- a/test/rubygems/test_gem_specification.rb +++ b/test/rubygems/test_gem_specification.rb @@ -2974,7 +2974,37 @@ def test_validate_name @a1.validate end - assert_equal 'invalid value for attribute name: ":json"', e.message + assert_equal 'invalid value for attribute name: ":json" must be a string', e.message + + @a1.name = [] + e = assert_raises Gem::InvalidSpecificationException do + @a1.validate + end + assert_equal "invalid value for attribute name: \"[]\" must be a string", e.message + + @a1.name = "" + e = assert_raises Gem::InvalidSpecificationException do + @a1.validate + end + assert_equal "invalid value for attribute name: \"\" must include at least one letter", e.message + + @a1.name = "12345" + e = assert_raises Gem::InvalidSpecificationException do + @a1.validate + end + assert_equal "invalid value for attribute name: \"12345\" must include at least one letter", e.message + + @a1.name = "../malicious" + e = assert_raises Gem::InvalidSpecificationException do + @a1.validate + end + assert_equal "invalid value for attribute name: \"../malicious\" can only include letters, numbers, dashes, and underscores", e.message + + @a1.name = "\ba\t" + e = assert_raises Gem::InvalidSpecificationException do + @a1.validate + end + assert_equal "invalid value for attribute name: \"\\ba\\t\" can only include letters, numbers, dashes, and underscores", e.message end def test_validate_non_nil diff --git a/test/rubygems/test_gem_text.rb b/test/rubygems/test_gem_text.rb index a6e22e04..04f3f605 100644 --- a/test/rubygems/test_gem_text.rb +++ b/test/rubygems/test_gem_text.rb @@ -36,6 +36,10 @@ def test_format_text_trailing # for two spaces after . assert_equal expected, format_text(text, 78) end + def test_format_removes_nonprintable_characters + assert_equal "text with weird .. stuff .", format_text("text with weird \x1b\x02 stuff \x7f", 40) + end + def test_min3 assert_equal 1, min3(1, 1, 1) assert_equal 1, min3(1, 1, 2) @@ -74,4 +78,11 @@ def test_levenshtein_distance_replace assert_equal 7, levenshtein_distance("xxxxxxx", "ZenTest") assert_equal 7, levenshtein_distance("zentest", "xxxxxxx") end + + def test_truncate_text + assert_equal "abc", truncate_text("abc", "desc") + assert_equal "Truncating desc to 2 characters:\nab", truncate_text("abc", "desc", 2) + s = "ab" * 500_001 + assert_equal "Truncating desc to 1,000,000 characters:\n#{s[0, 1_000_000]}", truncate_text(s, "desc", 1_000_000) + end end
From 61bccf2c72ec0ef185ebf43e1dd3cfc7c9b788d1 Mon Sep 17 00:00:00 2001 From: Florian Frank <fl...@ping.de> Date: Sat, 2 Sep 2017 15:07:45 -0300 Subject: [PATCH 1/4] Fix arbitrary heap exposure problem CVE-2017-14064 Backported for Ruby 2.3 by Hiroshi SHIBATA <h...@ruby-lang.org> https://bugs.ruby-lang.org/issues/13853 Applied in Debian by Antonio Terceiro <terce...@debian.org> --- ext/json/generator/generator.c | 12 ++++++------ ext/json/generator/generator.h | 1 - ext/json/lib/json/version.rb | 2 +- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/ext/json/generator/generator.c b/ext/json/generator/generator.c index a135e283..2cdca568 100644 --- a/ext/json/generator/generator.c +++ b/ext/json/generator/generator.c @@ -301,7 +301,7 @@ static char *fstrndup(const char *ptr, unsigned long len) { char *result; if (len <= 0) return NULL; result = ALLOC_N(char, len); - memccpy(result, ptr, 0, len); + memcpy(result, ptr, len); return result; } @@ -1055,7 +1055,7 @@ static VALUE cState_indent_set(VALUE self, VALUE indent) } } else { if (state->indent) ruby_xfree(state->indent); - state->indent = strdup(RSTRING_PTR(indent)); + state->indent = fstrndup(RSTRING_PTR(indent), len); state->indent_len = len; } return Qnil; @@ -1093,7 +1093,7 @@ static VALUE cState_space_set(VALUE self, VALUE space) } } else { if (state->space) ruby_xfree(state->space); - state->space = strdup(RSTRING_PTR(space)); + state->space = fstrndup(RSTRING_PTR(space), len); state->space_len = len; } return Qnil; @@ -1129,7 +1129,7 @@ static VALUE cState_space_before_set(VALUE self, VALUE space_before) } } else { if (state->space_before) ruby_xfree(state->space_before); - state->space_before = strdup(RSTRING_PTR(space_before)); + state->space_before = fstrndup(RSTRING_PTR(space_before), len); state->space_before_len = len; } return Qnil; @@ -1166,7 +1166,7 @@ static VALUE cState_object_nl_set(VALUE self, VALUE object_nl) } } else { if (state->object_nl) ruby_xfree(state->object_nl); - state->object_nl = strdup(RSTRING_PTR(object_nl)); + state->object_nl = fstrndup(RSTRING_PTR(object_nl), len); state->object_nl_len = len; } return Qnil; @@ -1201,7 +1201,7 @@ static VALUE cState_array_nl_set(VALUE self, VALUE array_nl) } } else { if (state->array_nl) ruby_xfree(state->array_nl); - state->array_nl = strdup(RSTRING_PTR(array_nl)); + state->array_nl = fstrndup(RSTRING_PTR(array_nl), len); state->array_nl_len = len; } return Qnil; diff --git a/ext/json/generator/generator.h b/ext/json/generator/generator.h index 298c0a49..6bbf817b 100644 --- a/ext/json/generator/generator.h +++ b/ext/json/generator/generator.h @@ -1,7 +1,6 @@ #ifndef _GENERATOR_H_ #define _GENERATOR_H_ -#include <string.h> #include <math.h> #include <ctype.h> diff --git a/ext/json/lib/json/version.rb b/ext/json/lib/json/version.rb index b5748334..cd7ddf87 100644 --- a/ext/json/lib/json/version.rb +++ b/ext/json/lib/json/version.rb @@ -1,7 +1,7 @@ # frozen_string_literal: false module JSON # JSON version - VERSION = '1.8.3' + VERSION = '1.8.3.1' VERSION_ARRAY = VERSION.split(/\./).map { |x| x.to_i } # :nodoc: VERSION_MAJOR = VERSION_ARRAY[0] # :nodoc: VERSION_MINOR = VERSION_ARRAY[1] # :nodoc: -- 2.14.1
From a58115188d90c257953ab80b8bf1619ce3c764d1 Mon Sep 17 00:00:00 2001
From: Antonio Terceiro <terce...@debian.org>
Date: Sat, 2 Sep 2017 15:14:24 -0300
Subject: [PATCH 2/4] Fix multiple security vulnerabilities
Security fixes:
Fix a DNS request hijacking vulnerability. Discovered by Jonathan Claudius, fix
by Samuel Giddins. (CVE-2017-0902)
Fix an ANSI escape sequence vulnerability. Discovered by Yusuke Endoh, fix by
Evan Phoenix. (CVE-2017-0899)
Fix a DOS vulernerability in the query command. Discovered by Yusuke Endoh, fix
by Samuel Giddins. (CVE-2017-0900)
Fix a vulnerability in the gem installer that allowed a malicious gem to
overwrite arbitrary files. Discovered by Yusuke Endoh, fix by Samuel Giddins.
(CVE-2017-0901)
---
lib/rubygems.rb | 2 +-
lib/rubygems/commands/query_command.rb | 5 +-
lib/rubygems/installer.rb | 7 +++
lib/rubygems/remote_fetcher.rb | 2 +-
lib/rubygems/specification.rb | 12 +++-
lib/rubygems/text.rb | 15 ++++-
test/rubygems/test_gem_commands_query_command.rb | 80 ++++++++++++++++++++++++
test/rubygems/test_gem_installer.rb | 20 ++++++
test/rubygems/test_gem_remote_fetcher.rb | 15 +++++
test/rubygems/test_gem_specification.rb | 32 +++++++++-
test/rubygems/test_gem_text.rb | 11 ++++
11 files changed, 193 insertions(+), 8 deletions(-)
diff --git a/lib/rubygems.rb b/lib/rubygems.rb
index 04031c76..9c0219ce 100644
--- a/lib/rubygems.rb
+++ b/lib/rubygems.rb
@@ -10,7 +10,7 @@
require 'thread'
module Gem
- VERSION = '2.5.2'
+ VERSION = '2.5.2.1'
end
# Must be first since it unloads the prelude from 1.9.2
diff --git a/lib/rubygems/commands/query_command.rb b/lib/rubygems/commands/query_command.rb
index d6196b44..61e98088 100644
--- a/lib/rubygems/commands/query_command.rb
+++ b/lib/rubygems/commands/query_command.rb
@@ -226,7 +226,7 @@ def output_versions output, versions
end
end
- output << make_entry(matching_tuples, platforms)
+ output << clean_text(make_entry(matching_tuples, platforms))
end
end
@@ -344,7 +344,8 @@ def spec_platforms entry, platforms
end
def spec_summary entry, spec
- entry << "\n\n" << format_text(spec.summary, 68, 4)
+ summary = truncate_text(spec.summary, "the summary for #{spec.full_name}")
+ entry << "\n\n" << format_text(summary, 68, 4)
end
end
diff --git a/lib/rubygems/installer.rb b/lib/rubygems/installer.rb
index 85358e0d..709b77d1 100644
--- a/lib/rubygems/installer.rb
+++ b/lib/rubygems/installer.rb
@@ -693,6 +693,11 @@ def verify_gem_home(unpack = false) # :nodoc:
unpack or File.writable?(gem_home)
end
+ def verify_spec_name
+ return if spec.name =~ Gem::Specification::VALID_NAME_PATTERN
+ raise Gem::InstallError, "#{spec} has an invalid name"
+ end
+
##
# Return the text for an application file.
@@ -812,6 +817,8 @@ def pre_install_checks
ensure_loadable_spec
+ verify_spec_name
+
if options[:install_as_default]
Gem.ensure_default_gem_subdirectories gem_home
else
diff --git a/lib/rubygems/remote_fetcher.rb b/lib/rubygems/remote_fetcher.rb
index fda1e067..254bebfa 100644
--- a/lib/rubygems/remote_fetcher.rb
+++ b/lib/rubygems/remote_fetcher.rb
@@ -104,7 +104,7 @@ def api_endpoint(uri)
else
target = res.target.to_s.strip
- if /\.#{Regexp.quote(host)}\z/ =~ target
+ if URI("http://"; + target).host.end_with?(".#{host}")
return URI.parse "#{uri.scheme}://#{target}#{uri.path}"
end
diff --git a/lib/rubygems/specification.rb b/lib/rubygems/specification.rb
index 9f635b1f..2519b96b 100644
--- a/lib/rubygems/specification.rb
+++ b/lib/rubygems/specification.rb
@@ -108,6 +108,8 @@ class Gem::Specification < Gem::BasicSpecification
private_constant :LOAD_CACHE if defined? private_constant
+ VALID_NAME_PATTERN = /\A[a-zA-Z0-9\.\-\_]+\z/ # :nodoc:
+
# :startdoc:
##
@@ -2667,9 +2669,15 @@ def validate packaging = true
end
end
- unless String === name then
+ if !name.is_a?(String) then
+ raise Gem::InvalidSpecificationException,
+ "invalid value for attribute name: \"#{name.inspect}\" must be a string"
+ elsif name !~ /[a-zA-Z]/ then
+ raise Gem::InvalidSpecificationException,
+ "invalid value for attribute name: #{name.dump} must include at least one letter"
+ elsif name !~ VALID_NAME_PATTERN then
raise Gem::InvalidSpecificationException,
- "invalid value for attribute name: \"#{name.inspect}\""
+ "invalid value for attribute name: #{name.dump} can only include letters, numbers, dashes, and underscores"
end
if raw_require_paths.empty? then
diff --git a/lib/rubygems/text.rb b/lib/rubygems/text.rb
index 732f1b99..b944b62c 100644
--- a/lib/rubygems/text.rb
+++ b/lib/rubygems/text.rb
@@ -6,13 +6,26 @@
module Gem::Text
+ ##
+ # Remove any non-printable characters and make the text suitable for
+ # printing.
+ def clean_text(text)
+ text.gsub(/[\000-\b\v-\f\016-\037\177]/, ".".freeze)
+ end
+
+ def truncate_text(text, description, max_length = 100_000)
+ raise ArgumentError, "max_length must be positive" unless max_length > 0
+ return text if text.size <= max_length
+ "Truncating #{description} to #{max_length.to_s.reverse.gsub(/...(?=.)/,'\&,').reverse} characters:\n" + text[0, max_length]
+ end
+
##
# Wraps +text+ to +wrap+ characters and optionally indents by +indent+
# characters
def format_text(text, wrap, indent=0)
result = []
- work = text.dup
+ work = clean_text(text)
while work.length > wrap do
if work =~ /^(.{0,#{wrap}})[ \n]/ then
diff --git a/test/rubygems/test_gem_commands_query_command.rb b/test/rubygems/test_gem_commands_query_command.rb
index 78c15a17..9ec71549 100644
--- a/test/rubygems/test_gem_commands_query_command.rb
+++ b/test/rubygems/test_gem_commands_query_command.rb
@@ -116,6 +116,86 @@ def test_execute_details
This is a lot of text. This is a lot of text. This is a lot of text.
This is a lot of text.
+pl (1)
+ Platform: i386-linux
+ Author: A User
+ Homepage: http://example.com
+
+ this is a summary
+ EOF
+
+ assert_equal expected, @ui.output
+ assert_equal '', @ui.error
+ end
+
+ def test_execute_details_cleans_text
+ spec_fetcher do |fetcher|
+ fetcher.spec 'a', 2 do |s|
+ s.summary = 'This is a lot of text. ' * 4
+ s.authors = ["Abraham Lincoln \x01", "\x02 Hirohito"]
+ s.homepage = "http://a.example.com/\x03";
+ end
+
+ fetcher.legacy_platform
+ end
+
+ @cmd.handle_options %w[-r -d]
+
+ use_ui @ui do
+ @cmd.execute
+ end
+
+ expected = <<-EOF
+
+*** REMOTE GEMS ***
+
+a (2)
+ Authors: Abraham Lincoln ., . Hirohito
+ Homepage: http://a.example.com/.
+
+ This is a lot of text. This is a lot of text. This is a lot of text.
+ This is a lot of text.
+
+pl (1)
+ Platform: i386-linux
+ Author: A User
+ Homepage: http://example.com
+
+ this is a summary
+ EOF
+
+ assert_equal expected, @ui.output
+ assert_equal '', @ui.error
+ end
+
+ def test_execute_details_truncates_summary
+ spec_fetcher do |fetcher|
+ fetcher.spec 'a', 2 do |s|
+ s.summary = 'This is a lot of text. ' * 10_000
+ s.authors = ["Abraham Lincoln \x01", "\x02 Hirohito"]
+ s.homepage = "http://a.example.com/\x03";
+ end
+
+ fetcher.legacy_platform
+ end
+
+ @cmd.handle_options %w[-r -d]
+
+ use_ui @ui do
+ @cmd.execute
+ end
+
+ expected = <<-EOF
+
+*** REMOTE GEMS ***
+
+a (2)
+ Authors: Abraham Lincoln ., . Hirohito
+ Homepage: http://a.example.com/.
+
+ Truncating the summary for a-2 to 100,000 characters:
+#{" This is a lot of text. This is a lot of text. This is a lot of text.\n" * 1449} This is a lot of te
+
pl (1)
Platform: i386-linux
Author: A User
diff --git a/test/rubygems/test_gem_installer.rb b/test/rubygems/test_gem_installer.rb
index 5ec71d0a..1092a0c6 100644
--- a/test/rubygems/test_gem_installer.rb
+++ b/test/rubygems/test_gem_installer.rb
@@ -1227,6 +1227,26 @@ def test_pre_install_checks_wrong_rubygems_version
end
end
+ def test_pre_install_checks_malicious_name
+ spec = util_spec '../malicious', '1'
+ def spec.full_name # so the spec is buildable
+ "malicious-1"
+ end
+ def spec.validate; end
+
+ util_build_gem spec
+
+ gem = File.join(@gemhome, 'cache', spec.file_name)
+
+ use_ui @ui do
+ @installer = Gem::Installer.at gem
+ e = assert_raises Gem::InstallError do
+ @installer.pre_install_checks
+ end
+ assert_equal '#<Gem::Specification name=../malicious version=1> has an invalid name', e.message
+ end
+ end
+
def test_shebang
util_make_exec @spec, "#!/usr/bin/ruby"
diff --git a/test/rubygems/test_gem_remote_fetcher.rb b/test/rubygems/test_gem_remote_fetcher.rb
index 49b6b665..a3919c8e 100644
--- a/test/rubygems/test_gem_remote_fetcher.rb
+++ b/test/rubygems/test_gem_remote_fetcher.rb
@@ -253,6 +253,21 @@ def test_api_endpoint_ignores_trans_domain_values_that_end_with_original
dns.verify
end
+ def test_api_endpoint_ignores_trans_domain_values_that_end_with_original_in_path
+ uri = URI.parse "http://example.com/foo";
+ target = MiniTest::Mock.new
+ target.expect :target, "evil.com/a.example.com"
+
+ dns = MiniTest::Mock.new
+ dns.expect :getresource, target, [String, Object]
+
+ fetch = Gem::RemoteFetcher.new nil, dns
+ assert_equal URI.parse("http://example.com/foo";), fetch.api_endpoint(uri)
+
+ target.verify
+ dns.verify
+ end
+
def test_api_endpoint_timeout_warning
uri = URI.parse "http://gems.example.com/foo";
diff --git a/test/rubygems/test_gem_specification.rb b/test/rubygems/test_gem_specification.rb
index bc1c8d2c..9a49bbbf 100644
--- a/test/rubygems/test_gem_specification.rb
+++ b/test/rubygems/test_gem_specification.rb
@@ -2974,7 +2974,37 @@ def test_validate_name
@a1.validate
end
- assert_equal 'invalid value for attribute name: ":json"', e.message
+ assert_equal 'invalid value for attribute name: ":json" must be a string', e.message
+
+ @a1.name = []
+ e = assert_raises Gem::InvalidSpecificationException do
+ @a1.validate
+ end
+ assert_equal "invalid value for attribute name: \"[]\" must be a string", e.message
+
+ @a1.name = ""
+ e = assert_raises Gem::InvalidSpecificationException do
+ @a1.validate
+ end
+ assert_equal "invalid value for attribute name: \"\" must include at least one letter", e.message
+
+ @a1.name = "12345"
+ e = assert_raises Gem::InvalidSpecificationException do
+ @a1.validate
+ end
+ assert_equal "invalid value for attribute name: \"12345\" must include at least one letter", e.message
+
+ @a1.name = "../malicious"
+ e = assert_raises Gem::InvalidSpecificationException do
+ @a1.validate
+ end
+ assert_equal "invalid value for attribute name: \"../malicious\" can only include letters, numbers, dashes, and underscores", e.message
+
+ @a1.name = "\ba\t"
+ e = assert_raises Gem::InvalidSpecificationException do
+ @a1.validate
+ end
+ assert_equal "invalid value for attribute name: \"\\ba\\t\" can only include letters, numbers, dashes, and underscores", e.message
end
def test_validate_non_nil
diff --git a/test/rubygems/test_gem_text.rb b/test/rubygems/test_gem_text.rb
index a6e22e04..04f3f605 100644
--- a/test/rubygems/test_gem_text.rb
+++ b/test/rubygems/test_gem_text.rb
@@ -36,6 +36,10 @@ def test_format_text_trailing # for two spaces after .
assert_equal expected, format_text(text, 78)
end
+ def test_format_removes_nonprintable_characters
+ assert_equal "text with weird .. stuff .", format_text("text with weird \x1b\x02 stuff \x7f", 40)
+ end
+
def test_min3
assert_equal 1, min3(1, 1, 1)
assert_equal 1, min3(1, 1, 2)
@@ -74,4 +78,11 @@ def test_levenshtein_distance_replace
assert_equal 7, levenshtein_distance("xxxxxxx", "ZenTest")
assert_equal 7, levenshtein_distance("zentest", "xxxxxxx")
end
+
+ def test_truncate_text
+ assert_equal "abc", truncate_text("abc", "desc")
+ assert_equal "Truncating desc to 2 characters:\nab", truncate_text("abc", "desc", 2)
+ s = "ab" * 500_001
+ assert_equal "Truncating desc to 1,000,000 characters:\n#{s[0, 1_000_000]}", truncate_text(s, "desc", 1_000_000)
+ end
end
--
2.14.1
From 019be7df6347302dfe586edf04bd298af05ad311 Mon Sep 17 00:00:00 2001
From: Shugo Maeda <sh...@ruby-lang.org>
Date: Wed, 8 Jun 2016 07:06:57 +0000
Subject: [PATCH 3/4] Fix SMTP command injection
[CVE-2015-9096]
* lib/net/smtp.rb (getok, get_response): raise an ArgumentError
when CR or LF is included in a line, because they are not
allowed in RFC5321.
git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@55324 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
---
lib/net/smtp.rb | 9 +++++++++
test/net/smtp/test_smtp.rb | 47 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 56 insertions(+)
diff --git a/lib/net/smtp.rb b/lib/net/smtp.rb
index d634274c..78f2181d 100644
--- a/lib/net/smtp.rb
+++ b/lib/net/smtp.rb
@@ -926,7 +926,15 @@ def quit
private
+ def validate_line(line)
+ # A bare CR or LF is not allowed in RFC5321.
+ if /[\r\n]/ =~ line
+ raise ArgumentError, "A line must not contain CR or LF"
+ end
+ end
+
def getok(reqline)
+ validate_line reqline
res = critical {
@socket.writeline reqline
recv_response()
@@ -936,6 +944,7 @@ def getok(reqline)
end
def get_response(reqline)
+ validate_line reqline
@socket.writeline reqline
recv_response()
end
diff --git a/test/net/smtp/test_smtp.rb b/test/net/smtp/test_smtp.rb
index 0edb3419..3bcceb6f 100644
--- a/test/net/smtp/test_smtp.rb
+++ b/test/net/smtp/test_smtp.rb
@@ -6,6 +6,8 @@
module Net
class TestSMTP < Test::Unit::TestCase
class FakeSocket
+ attr_reader :write_io
+
def initialize out = "250 OK\n"
@write_io = StringIO.new
@read_io = StringIO.new out
@@ -51,5 +53,50 @@ def test_rset
assert smtp.rset
end
+
+ def test_mailfrom
+ sock = FakeSocket.new
+ smtp = Net::SMTP.new 'localhost', 25
+ smtp.instance_variable_set :@socket, sock
+ assert smtp.mailfrom("f...@example.com").success?
+ assert_equal "MAIL FROM:<f...@example.com>\r\n", sock.write_io.string
+ end
+
+ def test_rcptto
+ sock = FakeSocket.new
+ smtp = Net::SMTP.new 'localhost', 25
+ smtp.instance_variable_set :@socket, sock
+ assert smtp.rcptto("f...@example.com").success?
+ assert_equal "RCPT TO:<f...@example.com>\r\n", sock.write_io.string
+ end
+
+ def test_auth_plain
+ sock = FakeSocket.new
+ smtp = Net::SMTP.new 'localhost', 25
+ smtp.instance_variable_set :@socket, sock
+ assert smtp.auth_plain("foo", "bar").success?
+ assert_equal "AUTH PLAIN AGZvbwBiYXI=\r\n", sock.write_io.string
+ end
+
+ def test_crlf_injection
+ smtp = Net::SMTP.new 'localhost', 25
+ smtp.instance_variable_set :@socket, FakeSocket.new
+
+ assert_raise(ArgumentError) do
+ smtp.mailfrom("foo\r\nbar")
+ end
+
+ assert_raise(ArgumentError) do
+ smtp.mailfrom("foo\rbar")
+ end
+
+ assert_raise(ArgumentError) do
+ smtp.mailfrom("foo\nbar")
+ end
+
+ assert_raise(ArgumentError) do
+ smtp.rcptto("foo\r\nbar")
+ end
+ end
end
end
--
2.14.1
From 668f3ec756efc826e0d976d37301fe6de9b0f38b Mon Sep 17 00:00:00 2001
From: Kazuki Yamaguchi <k...@rhe.jp>
Date: Wed, 16 Nov 2016 01:45:11 +0000
Subject: [PATCH 4/4] cipher: don't set dummy encryption key in
Cipher#initialize
Remove the encryption key initialization from Cipher#initialize. This
is effectively a revert of r32723 ("Avoid possible SEGV from AES
encryption/decryption", 2011-07-28).
r32723, which added the key initialization, was a workaround for
Ruby Bug #2768. For some certain ciphers, calling EVP_CipherUpdate()
before setting an encryption key caused segfault. It was not a problem
until OpenSSL implemented GCM mode - the encryption key could be
overridden by repeated calls of EVP_CipherInit_ex(). But, it is not the
case for AES-GCM ciphers. Setting a key, an IV, a key, in this order
causes the IV to be reset to an all-zero IV.
The problem of Bug #2768 persists on the current versions of OpenSSL.
So, make Cipher#update raise an exception if a key is not yet set by the
user. Since encrypting or decrypting without key does not make any
sense, this should not break existing applications.
Users can still call Cipher#key= and Cipher#iv= multiple times with
their own responsibility.
Reference: https://bugs.ruby-lang.org/issues/2768
Reference: https://bugs.ruby-lang.org/issues/8221
Reference: https://github.com/ruby/openssl/issues/49
Closes: #842432 [CVE-2016-7798]
Backportted for Debian by Christian Hofstaedtler <z...@debian.org>
Signed-off-by: Christian Hofstaedtler <z...@debian.org>
Signed-off-by: Antonio Terceiro <terce...@debian.org>
---
ext/openssl/ossl_cipher.c | 22 +++++++++++++---------
test/openssl/test_cipher.rb | 29 +++++++++++++++++++++++------
2 files changed, 36 insertions(+), 15 deletions(-)
diff --git a/ext/openssl/ossl_cipher.c b/ext/openssl/ossl_cipher.c
index 09b021d9..24b84679 100644
--- a/ext/openssl/ossl_cipher.c
+++ b/ext/openssl/ossl_cipher.c
@@ -34,6 +34,7 @@
*/
VALUE cCipher;
VALUE eCipherError;
+static ID id_key_set;
static VALUE ossl_cipher_alloc(VALUE klass);
static void ossl_cipher_free(void *ptr);
@@ -114,7 +115,6 @@ ossl_cipher_initialize(VALUE self, VALUE str)
EVP_CIPHER_CTX *ctx;
const EVP_CIPHER *cipher;
char *name;
- unsigned char key[EVP_MAX_KEY_LENGTH];
name = StringValuePtr(str);
GetCipherInit(self, ctx);
@@ -126,14 +126,7 @@ ossl_cipher_initialize(VALUE self, VALUE str)
if (!(cipher = EVP_get_cipherbyname(name))) {
ossl_raise(rb_eRuntimeError, "unsupported cipher algorithm (%s)", name);
}
- /*
- * The EVP which has EVP_CIPH_RAND_KEY flag (such as DES3) allows
- * uninitialized key, but other EVPs (such as AES) does not allow it.
- * Calling EVP_CipherUpdate() without initializing key causes SEGV so we
- * set the data filled with "\0" as the key by default.
- */
- memset(key, 0, EVP_MAX_KEY_LENGTH);
- if (EVP_CipherInit_ex(ctx, cipher, NULL, key, NULL, -1) != 1)
+ if (EVP_CipherInit_ex(ctx, cipher, NULL, NULL, NULL, -1) != 1)
ossl_raise(eCipherError, NULL);
return self;
@@ -252,6 +245,9 @@ ossl_cipher_init(int argc, VALUE *argv, VALUE self, int mode)
ossl_raise(eCipherError, NULL);
}
+ if (p_key)
+ rb_ivar_set(self, id_key_set, Qtrue);
+
return self;
}
@@ -338,6 +334,8 @@ ossl_cipher_pkcs5_keyivgen(int argc, VALUE *argv, VALUE self)
OPENSSL_cleanse(key, sizeof key);
OPENSSL_cleanse(iv, sizeof iv);
+ rb_ivar_set(self, id_key_set, Qtrue);
+
return Qnil;
}
@@ -390,6 +388,8 @@ ossl_cipher_update(int argc, VALUE *argv, VALUE self)
VALUE data, str;
rb_scan_args(argc, argv, "11", &data, &str);
+ if (!RTEST(rb_attr_get(self, id_key_set)))
+ ossl_raise(eCipherError, "key not set");
StringValue(data);
in = (unsigned char *)RSTRING_PTR(data);
@@ -490,6 +490,8 @@ ossl_cipher_set_key(VALUE self, VALUE key)
if (EVP_CipherInit_ex(ctx, NULL, NULL, (unsigned char *)RSTRING_PTR(key), NULL, -1) != 1)
ossl_raise(eCipherError, NULL);
+ rb_ivar_set(self, id_key_set, Qtrue);
+
return key;
}
@@ -1008,4 +1010,6 @@ Init_ossl_cipher(void)
rb_define_method(cCipher, "iv_len", ossl_cipher_iv_length, 0);
rb_define_method(cCipher, "block_size", ossl_cipher_block_size, 0);
rb_define_method(cCipher, "padding=", ossl_cipher_set_padding, 1);
+
+ id_key_set = rb_intern_const("key_set");
}
diff --git a/test/openssl/test_cipher.rb b/test/openssl/test_cipher.rb
index 89c176f4..fb08b610 100644
--- a/test/openssl/test_cipher.rb
+++ b/test/openssl/test_cipher.rb
@@ -81,6 +81,7 @@ def test_reset
def test_empty_data
@c1.encrypt
+ @c1.random_key
assert_raise(ArgumentError){ @c1.update("") }
end
@@ -129,12 +130,10 @@ def test_AES
}
end
- def test_AES_crush
- 500.times do
- assert_nothing_raised("[Bug #2768]") do
- # it caused OpenSSL SEGV by uninitialized key
- OpenSSL::Cipher::AES128.new("ECB").update "." * 17
- end
+ def test_update_raise_if_key_not_set
+ assert_raise(OpenSSL::Cipher::CipherError) do
+ # it caused OpenSSL SEGV by uninitialized key
+ OpenSSL::Cipher::AES128.new("ECB").update "." * 17
end
end
end
@@ -238,6 +237,24 @@ def test_aes_gcm_wrong_ciphertext
end
+ def test_aes_gcm_key_iv_order_issue
+ pt = "[ruby/openssl#49]"
+ cipher = OpenSSL::Cipher.new("aes-128-gcm").encrypt
+ cipher.key = "x" * 16
+ cipher.iv = "a" * 12
+ ct1 = cipher.update(pt) << cipher.final
+ tag1 = cipher.auth_tag
+
+ cipher = OpenSSL::Cipher.new("aes-128-gcm").encrypt
+ cipher.iv = "a" * 12
+ cipher.key = "x" * 16
+ ct2 = cipher.update(pt) << cipher.final
+ tag2 = cipher.auth_tag
+
+ assert_equal ct1, ct2
+ assert_equal tag1, tag2
+ end if has_cipher?("aes-128-gcm")
+
private
def new_encryptor(algo)
--
2.14.1
signature.asc
Description: PGP signature
