Skip to content

Commit

Permalink
SECBUG-240 Fix out-of-bounds reads (#325)
Browse files Browse the repository at this point in the history
* final tweaks for BSON 5 release

* SECBUG-240 strlen might read beyond end of buffer

* ruby 2.6 isn't supported

* nix 2.6
  • Loading branch information
jamis authored Mar 28, 2024
1 parent 04602ac commit d3eab30
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 13 deletions.
23 changes: 17 additions & 6 deletions .evergreen/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ timeout:
script: |
ls -la
# -----------------------------------------------
# .evergreen/config/functions.yml.erb
# -----------------------------------------------
Expand Down Expand Up @@ -240,6 +241,7 @@ tasks:
commands:
- func: "run tests"


# -----------------------------------------------
# .evergreen/config/axes.yml.erb
# -----------------------------------------------
Expand Down Expand Up @@ -276,34 +278,42 @@ axes:
- id: ruby
display_name: Ruby Version
values:

- id: ruby-3.2
display_name: ruby-3.2
variables:
RVM_RUBY: ruby-3.2

- id: ruby-3.1
display_name: ruby-3.1
variables:
RVM_RUBY: ruby-3.1

- id: ruby-3.0
display_name: ruby-3.0
variables:
RVM_RUBY: ruby-3.0

- id: ruby-2.7
display_name: ruby-2.7
variables:
RVM_RUBY: ruby-2.7

- id: ruby-2.6
display_name: ruby-2.6
variables:
RVM_RUBY: ruby-2.6

- id: jruby-9.4
display_name: jruby-9.4
variables:
RVM_RUBY: jruby-9.4

- id: jruby-9.3
display_name: jruby-9.3
variables:
RVM_RUBY: jruby-9.3
- id: jruby-9.2
display_name: jruby-9.2
variables:
RVM_RUBY: jruby-9.2


- id: "as"
display_name: ActiveSupport
Expand Down Expand Up @@ -337,6 +347,7 @@ axes:
variables:
COMPACT: true


# -----------------------------------------------
# .evergreen/config/variants.yml.erb
# -----------------------------------------------
Expand Down Expand Up @@ -373,13 +384,13 @@ buildvariants:
- name: "test"

- matrix_name: "special-os"
matrix_spec: { ruby: ["ruby-3.2", "ruby-3.1", "jruby-9.3"], special-os: '*' }
matrix_spec: { ruby: ["ruby-3.2", "ruby-3.1", "jruby-9.4"], special-os: '*' }
display_name: "${ruby}, ${special-os}"
tasks:
- name: "test"

- matrix_name: "jruby"
matrix_spec: { ruby: ["jruby-9.3", "jruby-9.2"], all-os: rhel }
matrix_spec: { ruby: ["jruby-9.4", "jruby-9.3"], all-os: rhel }
display_name: "${ruby}, ${all-os}"
tasks:
- name: "test"
Expand Down
6 changes: 3 additions & 3 deletions .evergreen/update-evergreen-configs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ module ConfigProcessor

# these are used for testing against a few recent ruby versions
def recent_rubies
@recent_rubies ||= %w[ ruby-3.2 ruby-3.1 jruby-9.3 ]
@recent_rubies ||= %w[ ruby-3.2 ruby-3.1 jruby-9.4 ]
end

# the most recently released, stable version of Ruby (make sure this
Expand All @@ -96,7 +96,7 @@ module ConfigProcessor

# as above, but including the most recent JRuby release
def sample_rubies
@sample_rubies ||= sample_mri_rubies + %w[ jruby-9.3 ]
@sample_rubies ||= sample_mri_rubies + %w[ jruby-9.4 ]
end

# older Ruby versions provided by 10gen/mongo-ruby-toolchain
Expand All @@ -106,7 +106,7 @@ module ConfigProcessor

# all supported JRuby versions provided by 10gen/mongo-ruby-toolchain
def jrubies
@jrubies ||= %w[ jruby-9.3 jruby-9.2 ]
@jrubies ||= %w[ jruby-9.4 jruby-9.3 ]
end

# all supported MRI ruby versions
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/bson-ruby.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
fail-fast: false
matrix:
os: [ ubuntu, macos, windows ]
ruby: [ 2.6, 2.7, 3.0, 3.1, head ]
ruby: [ 2.7, 3.0, 3.1, 3.2, 3.3, head ]
include:
- { os: windows , ruby: mingw }
exclude:
Expand Down
21 changes: 18 additions & 3 deletions ext/bson/read.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ static VALUE pvt_get_symbol(byte_buffer_t *b, VALUE rb_buffer, int argc, VALUE *
static VALUE pvt_get_boolean(byte_buffer_t *b);
static VALUE pvt_read_field(byte_buffer_t *b, VALUE rb_buffer, uint8_t type, int argc, VALUE *argv);
static void pvt_skip_cstring(byte_buffer_t *b);
static size_t pvt_strnlen(const byte_buffer_t *b);

void pvt_raise_decode_error(volatile VALUE msg) {
VALUE klass = pvt_const_get_3("BSON", "Error", "BSONDecodeError");
Expand Down Expand Up @@ -143,7 +144,7 @@ VALUE rb_bson_byte_buffer_get_bytes(VALUE self, VALUE i)
}

VALUE pvt_get_boolean(byte_buffer_t *b){
VALUE result;
VALUE result = Qnil;
char byte_value;
ENSURE_BSON_READ(b, 1);
byte_value = *READ_PTR(b);
Expand Down Expand Up @@ -236,7 +237,7 @@ VALUE rb_bson_byte_buffer_get_cstring(VALUE self)
int length;

TypedData_Get_Struct(self, byte_buffer_t, &rb_byte_buffer_data_type, b);
length = (int)strlen(READ_PTR(b));
length = (int)pvt_strnlen(b);
ENSURE_BSON_READ(b, length);
string = rb_enc_str_new(READ_PTR(b), length, rb_utf8_encoding());
b->read_position += length + 1;
Expand All @@ -249,7 +250,7 @@ VALUE rb_bson_byte_buffer_get_cstring(VALUE self)
void pvt_skip_cstring(byte_buffer_t *b)
{
int length;
length = (int)strlen(READ_PTR(b));
length = (int)pvt_strnlen(b);
ENSURE_BSON_READ(b, length);
b->read_position += length + 1;
}
Expand Down Expand Up @@ -453,3 +454,17 @@ VALUE rb_bson_byte_buffer_get_array(int argc, VALUE *argv, VALUE self){

return array;
}

/**
* Returns the length of the given string `str`. If no null-terminating byte
* is present when `len` bytes have been scanned, then `len` is
* returned.
*/
size_t pvt_strnlen(const byte_buffer_t *b) {
const char *ptr = memchr(READ_PTR(b), '\0', READ_SIZE(b));

if (!ptr)
rb_raise(rb_eRangeError, "string is too long (possibly not null-terminated)");

return (size_t)(ptr - READ_PTR(b));
}
4 changes: 4 additions & 0 deletions ext/bson/write.c
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,10 @@ void pvt_put_array_index(byte_buffer_t *b, int32_t index)
c_str = buffer;
snprintf(buffer, sizeof(buffer), "%d", index);
}
// strlen is a potential vector for out-of-bounds errors, but
// the only way for that to happen here, specifically, is if `index`
// is greater than 10e16 - 1, which is far beyond the domain of an
// int32.
length = strlen(c_str) + 1;
ENSURE_BSON_WRITE(b, length);
memcpy(WRITE_PTR(b), c_str, length);
Expand Down

0 comments on commit d3eab30

Please sign in to comment.