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

rbnacl: fix byffer size type in randombytes_buf binding #174

Merged
merged 1 commit into from Apr 6, 2018
Merged

rbnacl: fix byffer size type in randombytes_buf binding #174

merged 1 commit into from Apr 6, 2018

Conversation

trofi
Copy link

@trofi trofi commented Mar 18, 2018

https://github.com/jedisct1/libsodium/blob/master/src/libsodium/include/sodium/randombytes.h#L35
defines randombytes_buf buffer size as size_t:

void randombytes_buf(void * const buf, const size_t size);

but rbnacl defines it as unsigned long long:

     sodium_function :c_random_bytes,
                     :randombytes_buf,
                     %i[pointer ulong_long]

This manifests as an error on 32-bit big-endian platforms because size_t
is 32-bit there and unsigned long long is 64-bit.

I've found it as a testsuite crash on powerpc32:

  #1  0x0fb93580 in abort () from /lib/libc.so.6
  #2  0x0f622da8 in sodium_misuse () at /dev/shm/portage/dev-libs/libsodium-1.0.16-r1/work/libsodium-1.0.16/src/libsodium/sodium/core.c:216
  #3  0x0f629d8c in randombytes_sysrandom_buf (buf=0x110a68f0, size=258618072)
      at /dev/shm/portage/dev-libs/libsodium-1.0.16-r1/work/libsodium-1.0.16/src/libsodium/randombytes/sysrandom/randombytes_sysrandom.c:337
  #4  0x0f621e3c in randombytes_buf (buf=<optimized out>, size=<optimized out>)
      at /dev/shm/portage/dev-libs/libsodium-1.0.16-r1/work/libsodium-1.0.16/src/libsodium/randombytes/randombytes.c:155
  #5  0x0f664d38 in ?? () from /usr/lib/libffi.so.6
  #6  0x0f663ee8 in ffi_call () from /usr/lib/libffi.so.6
  #7  0x0f6a3c98 in rbffi_CallFunction (argc=<optimized out>, argv=<optimized out>, function=0xf621df0 <randombytes_buf>, fnInfo=0x10e5f790) at Call.c:409
  #8  0x0f69eba0 in attached_method_invoke (cif=<optimized out>, mretval=0xfff929e0, parameters=<optimized out>, user_data=<optimized out>) at MethodHandle.c:65

Note how randombytes_sysrandom_buf accepts big size=258618072.
The fix is simple: use :size_t from ruby's ffi package instead.

This change makes all tests pass on powerpc32.

Signed-off-by: Sergei Trofimovich slyfox@gentoo.org

https://github.com/jedisct1/libsodium/blob/master/src/libsodium/include/sodium/randombytes.h#L35
defines `randombytes_buf` buffer size as `size_t`:

```c
void randombytes_buf(void * const buf, const size_t size);
```

but rbnacl defines it as `unsigned long long`:

```ruby
     sodium_function :c_random_bytes,
                     :randombytes_buf,
                     %i[pointer ulong_long]
```

This manifests as an error on 32-bit big-endian platforms because `size_t`
is 32-bit there and `unsigned long long` is 64-bit.

I've found it as a testsuite crash on powerpc32:

```
  #1  0x0fb93580 in abort () from /lib/libc.so.6
  #2  0x0f622da8 in sodium_misuse () at /dev/shm/portage/dev-libs/libsodium-1.0.16-r1/work/libsodium-1.0.16/src/libsodium/sodium/core.c:216
  #3  0x0f629d8c in randombytes_sysrandom_buf (buf=0x110a68f0, size=258618072)
      at /dev/shm/portage/dev-libs/libsodium-1.0.16-r1/work/libsodium-1.0.16/src/libsodium/randombytes/sysrandom/randombytes_sysrandom.c:337
  #4  0x0f621e3c in randombytes_buf (buf=<optimized out>, size=<optimized out>)
      at /dev/shm/portage/dev-libs/libsodium-1.0.16-r1/work/libsodium-1.0.16/src/libsodium/randombytes/randombytes.c:155
  #5  0x0f664d38 in ?? () from /usr/lib/libffi.so.6
  #6  0x0f663ee8 in ffi_call () from /usr/lib/libffi.so.6
  #7  0x0f6a3c98 in rbffi_CallFunction (argc=<optimized out>, argv=<optimized out>, function=0xf621df0 <randombytes_buf>, fnInfo=0x10e5f790) at Call.c:409
  #8  0x0f69eba0 in attached_method_invoke (cif=<optimized out>, mretval=0xfff929e0, parameters=<optimized out>, user_data=<optimized out>) at MethodHandle.c:65
```

Note how `randombytes_sysrandom_buf` accepts big `size=258618072`.
The fix is simple: use `:size_t` from ruby's ffi package instead.

This change makes all tests pass on powerpc32.

Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
@tarcieri tarcieri merged commit b28f4fd into RubyCrypto:master Apr 6, 2018
@tarcieri
Copy link
Contributor

tarcieri commented Apr 6, 2018

Thanks!

@tarcieri tarcieri mentioned this pull request Nov 8, 2018
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Dec 17, 2018
## [6.0.0] (2018-11-08)

[6.0.0]: RubyCrypto/rbnacl#182

* [#180](RubyCrypto/rbnacl#180)
  Deprecate rbnacl-libsodium.
  ([@tarcieri])

* [#176](RubyCrypto/rbnacl#176)
  Add support for XChaCha20-Poly1305.
  ([@AnIrishDuck])

* [#174](RubyCrypto/rbnacl#174)
  Fix buffer size type in `randombytes_buf` binding.
  ([@elijh])

* [#172](RubyCrypto/rbnacl#172)
  Add support for argon2id digest.
  ([@trofi])

* [#166](RubyCrypto/rbnacl#166)
  Support for non-32-byte HMAC-SHA256/512 keys.
  ([@nsheremet])
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants