Skip to content

Validate strings before native access #42

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.

Sign up for GitHub

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

Open
OskarEichler wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Validate strings before native access #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 10 additions & 4 deletions ext/mri/bcrypt_pbkdf_ext.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,18 @@ static VALUE cBCryptPbkdfEngine;
/* Given a secret and a salt a key and the number of rounds and returns the encrypted secret
*/
static VALUE bc_crypt_pbkdf(VALUE self, VALUE pass, VALUE salt, VALUE keylen, VALUE rounds) {
StringValue(pass);
StringValue(salt);

size_t okeylen = NUM2ULONG(keylen);
if (okeylen == 0 || okeylen > 1024)
return Qnil;
u_int8_t* okey = xmalloc(okeylen);
VALUE out;

int ret = bcrypt_pbkdf(
StringValuePtr(pass), RSTRING_LEN(pass),
(const u_int8_t*)StringValuePtr(salt), RSTRING_LEN(salt),
RSTRING_PTR(pass), RSTRING_LEN(pass),
(const u_int8_t*)RSTRING_PTR(salt), RSTRING_LEN(salt),
okey, okeylen,
NUM2ULONG(rounds));
if (ret < 0) {
Expand All @@ -29,11 +32,14 @@ static VALUE bc_crypt_pbkdf(VALUE self, VALUE pass, VALUE salt, VALUE keylen, VA

static VALUE bc_crypt_hash(VALUE self, VALUE pass, VALUE salt) {
u_int8_t hash[BCRYPT_HASHSIZE];
StringValue(pass);
StringValue(salt);

if (RSTRING_LEN(pass) != 64U)
return Qnil;
if (RSTRING_LEN(salt) != 64U)
return Qnil;
bcrypt_hash((u_int8_t*)StringValuePtr(pass), (u_int8_t*)StringValuePtr(salt), hash);
bcrypt_hash((u_int8_t*)RSTRING_PTR(pass), (u_int8_t*)RSTRING_PTR(salt), hash);
return rb_str_new((const char*)hash, sizeof(hash));
}

Expand All @@ -45,4 +51,4 @@ void Init_bcrypt_pbkdf_ext(){

rb_define_singleton_method(cBCryptPbkdfEngine, "__bc_crypt_pbkdf", bc_crypt_pbkdf, 4);
rb_define_singleton_method(cBCryptPbkdfEngine, "__bc_crypt_hash", bc_crypt_hash, 2);
}
}
11 changes: 11 additions & 0 deletions test/bcrypt_pnkdf/engine_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,17 @@ def test_hash_argument_lengths
assert_nil BCryptPbkdf::Engine.__bc_crypt_hash(sha2pass, sha2salt.byteslice(0, 63))
end

def test_string_arguments_are_validated_before_native_access
pass = Object.new
pass.define_singleton_method(:to_str) { "pass" }
salt = Object.new
salt.define_singleton_method(:to_str) { "salt" }

assert_equal BCryptPbkdf.key("pass", "salt", 32, 4), BCryptPbkdf.key(pass, salt, 32, 4)
assert_raises(TypeError) { BCryptPbkdf::Engine.__bc_crypt_hash(false, "s" * 64) }
assert_raises(TypeError) { BCryptPbkdf::Engine.__bc_crypt_hash("p" * 64, false) }
end

# Issue #31/33: xmalloc(okeylen) was called before the guards inside
# bcrypt_pbkdf(), so out-of-range keylen caused a heap allocation that was
# never freed when bcrypt_pbkdf() returned -1. `loop { key("p","s",2000,1) }`
Expand Down