Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SECBUG-240 Fix out-of-bounds reads #325

Merged
merged 5 commits into from
Mar 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading