0.21.1 (current)
From EmbarkStudios/rust-ecosystem. By Embark.
These reviews are from cargo-vet. To add your review, set up cargo-vet and submit your URL to its registry.
0.21.1 (current)
From kornelski/crev-proofs copy of git.savannah.gnu.org.
Packaged for Guix (crates-io)
cargo-vet does not verify reviewers' identity. You have to fully trust the source the audits are from.
- safe-to-deploy (implies safe-to-run)
-
This crate will not introduce a serious security vulnerability to production software exposed to untrusted input. More…
- safe-to-run
Implied by other criteria
This crate can be compiled, run, and tested on a local workstation or in controlled automation without surprising consequences. More…
- unknown
-
May have been packaged automatically without a review
This review is from Crev, a distributed system for code reviews. To add your review, set up cargo-crev.
The current version of JNI is 0.21.1.
0.13.0 (older version) Thoroughness: Low Understanding: Medium
by MaulingMonkey on 2019-07-31
Rated files were at least reviewed to thoroughness + understanding medium, but the rest was only reviewed to througuhness low.
Detail
| File | Rating | Notes |
|---|---|---|
| .github/PULL_REQUEST_TEMPLATE.md | +1 | |
| .travis/run_travis_job.sh | +1 | |
| .vscode/tasks.json | +1 | |
| benches/api_calls.rs | +1 | Various API concerns but this file is fine. |
| examples/HelloWorld.h | +1 | Matches HelloWorld.java... generated by javah? |
| examples/HelloWorld.java | +1 | |
| examples/Makefile | +1 | |
| src/wrapper/descriptors/class_desc.rs | +1 | |
| src/wrapper/descriptors/desc.rs | +1 | |
| src/wrapper/descriptors/exception_desc.rs | +1 | |
| src/wrapper/descriptors/field_desc.rs | +1 | |
| src/wrapper/descriptors/method_desc.rs | +1 | |
| src/wrapper/descriptors/mod.rs | +1 | |
| src/wrapper/java_vm/init_args.rs | +1 | |
| src/wrapper/java_vm/mod.rs | +1 | |
| src/wrapper/java_vm/vm.rs | -1 | Not 100% sure if it's sound to detatch threads out from under other Java code. Some unsafe attach fns also confuse me as to why they're unsafe. |
| src/wrapper/objects/auto_local.rs | -1 | Not 100% sure if AutoLocal::new is sound based on scary JVM crash warnings |
| src/wrapper/objects/global_ref.rs | +1 | |
| src/wrapper/objects/jbytebuffer.rs | -1 | Allowing construction from arbitrary jobject s will likely be unsound later |
| src/wrapper/objects/jclass.rs | -1 | Ditto |
| src/wrapper/objects/jfieldid.rs | -1 | Ditto |
| src/wrapper/objects/jlist.rs | -1 | Called it - internal is used in safe fns, unsound looking as fuck. |
| src/wrapper/objects/jmap.rs | ||
| src/wrapper/objects/jmethodid.rs | ||
| src/wrapper/objects/jobject.rs | ||
| src/wrapper/objects/jstaticfieldid.rs | ||
| src/wrapper/objects/jstaticmethodid.rs | ||
| src/wrapper/objects/jstring.rs | ||
| src/wrapper/objects/jthrowable.rs | ||
| src/wrapper/objects/jvalue.rs | ||
| src/wrapper/objects/mod.rs | ||
| src/wrapper/strings/ffi_str.rs | ||
| src/wrapper/strings/java_str.rs | ||
| src/wrapper/strings/mod.rs | ||
| src/wrapper/errors.rs | ||
| src/wrapper/executor.rs | ||
| src/wrapper/jnienv.rs | ||
| src/wrapper/macros.rs | ||
| src/wrapper/signature.rs | ||
| src/wrapper/version.rs | ||
| src/lib.rs | ||
| src/sys.rs | +1 | pub use jni_sys::* |
| tests/util/example_proxy.rs | ||
| tests/util/mod.rs | ||
| tests/executor_nested_attach.rs | ||
| tests/executor.rs | ||
| tests/java_integers.rs | ||
| tests/jmap.rs | ||
| tests/jni_api.rs | ||
| tests/jni_global_ref_is_deleted.rs | ||
| tests/jni_global_refs.rs | ||
| tests/threads_attach_guard.rs | ||
| tests/threads_detach_daemon.rs | ||
| tests/threads_detach.rs | ||
| tests/threads_explicit_detach_daemon.rs | ||
| tests/threads_explicit_detach_permanent.rs | ||
| tests/threads_explicit_detach.rs | ||
| tests/threads_nested_attach_daemon.rs | ||
| tests/threads_nested_attach_guard.rs | ||
| tests/threads_nested_attach_permanently.rs | ||
| .appveyor.yml | ||
| .gitignore | ||
| .travis.yml | ||
| Cargo.toml | +1 | |
| Cargo.toml.orig | +1 | |
| CHANGELOG.md | +1 | |
| clippy.toml | +1 | |
| CODE_OF_CONDUCT.md | +1 | |
| CONTRIBUTING.md | +1 | |
| LICENSE-APACHE | +1 | |
| LICENSE-MIT | +1 | |
| README.md | +1 |
| Other | Rating | Notes |
|---|---|---|
| unsafe | -1 | Unsound |
| fs | +1 | Not used |
| io | +1 | Not used |
| docs | +1 | Well documented |
| tests | +1 | Decent testing |
benches/api_calls.rs
| Line | Notes |
|---|---|
| 52 | ..._unchecked is safe? Look at call_static_method_unchecked carefully. |
| 69 | Not all ..._unchecked are safe? |
| 154 | Must manually drop local refs? Lame. |
| 226 | No use of black box? |
| +1 |
src/wrapper/descriptors/method_desc.rs
| Line | Notes |
|---|---|
| 24 | I feel like having an implicit "<init>" instead of a struct of some sort is potentially confusing? |
| +1 |
src/wrapper/java_vm/init_args.rs
| Line | Notes |
|---|---|
| 46 | Could use more doc-tests |
| 50 | Silently ignoring unsupported options is a little lame |
| 70 | JavaVM::build in doc comments, not new ? |
| 101 | Pretty gosh darn heckin' sketchy if you ask me... relies on opts never being modified after this point. Fortunately this type's contents are nice and private/local, so that's enforced. |
| +1 |
src/wrapper/java_vm/vm.rs
| Line | Notes |
|---|---|
| 134 | unsafe impl Send + Sync - I believe this is safe for JavaVM (as used here), but not for JNIEnv (keep a look out for that later) |
| 150 | unsafe { ... } - ptr casts are a bit sketchy, otherwise LGTM. |
| 158 | +1 |
| 165 | unsafe fn - +1 |
| 185 | attach_current_thread_permanently - possible noop if already attached, meaning it might be temporary! |
| 232 | detach_current_thread - doc comments make this sound possibly unsound? |
| 270 | unsafe { ... } - +1 |
| 280 | unsafe { ... } - +1 |
| 364 | unsafe fn - This... actually looks sound? What am I missing? |
| 386 | unsafe fn - This... actually looks sound? What am I missing? |
| 409 | unsafe { ... } - +1 |
| -1 - Can detatched threads cause unsoundness? What am I missing for unsafe fn? |
src/wrapper/objects/global_ref.rs
| Line | Notes |
|---|---|
| 36 | unsafe impl Send + Sync - should be safe? |
| 48 | unsafe fn - presumably because this takes a jobject. LGTM. |
| 66 | unsafe fn - presumably because this takes a jobject. LGTM. |
| +1 |
src/wrapper/objects/jbytebuffer.rs
| Line | Notes |
|---|---|
| 11 | jobject is just a pointer, so this general purpouse crate-exported method means using JByteBuffer s is unsafe! |
| 32 | there's no guarantee a given JObject is a JByteBuffer, but this succeeds unconditionally. |
| -1 - I suspect invalid jobject s will cause soundness issues later |
src/wrapper/objects/jlist.rs
| Line | Notes |
|---|---|
| 20 | Eww, methods cached per-object? |
| 46 | jobject -> JObject -> JList can be constructed with an invalid pointer... |
| 69 | ...making all safe fns on this type unsound. Use of 'safe' _unchecked methods also concerns me. |
| -1 |
src/wrapper/macros.rs
| Line | Rating | Notes |
|---|---|---|
| ... | ||
| 26 | +1 | non_null |
| ... | ||
| 105 | +1 | java_vm_unchecked - 'unchecked' refers to error codes. unsafe macro, $java_vm must be valid. |
| 132 | -1 | java_vm_method - I wish this forced the caller to use unsafe { ... }. unsafe macro, $jnienv must be valid. |
| ... |
TIL
- Use
javahto generate rust, perhaps? [build-dependencies]
-
Issue: High (github.com/jni-rs/jni-rs/issues/197)
Definite soundness issue
Lib.rs has been able to verify that all files in the crate's tarball are in the crate's repository with a git tag matching the version. Please note that this check is still in beta, and absence of this confirmation does not mean that the files don't match.
Crates in the crates.io registry are tarball snapshots uploaded by crates' publishers. The registry is not using crates' git repositories, so there is a possibility that published crates have a misleading repository URL, or contain different code from the code in the repository.
To review the actual code of the crate, it's best to use cargo crev open jni. Alternatively, you can download the tarball of jni v0.21.1 or view the source online.
Aims to provide a safe JNI (Java Native Interface) API over the unsafe
jni_syscrate.This is a very general FFI abstraction for Java VMs with a lot of unsafe code throughout the API. There are almost certainly some edge cases with its design that could lead to unsound behaviour but it should still be considerably safer than working with JNI directly.
A lot of the unsafe usage relates to quite-simple use of
from_rawAPIs to construct or cast wrapper types (around JNI pointers) which are fairly straight-forward to verify/trust in context.Some unsafe code has good
// # Safetydocumentation (this has been enforced for newer code) but a lot of unsafe code doesn't document invariants that are being relied on.The design depends on non-trivial named lifetimes across many APIs to associate Java local references with JNI stack frames.
The crate is not very actively maintained and was practically unmaintained for over a year before the 0.20 release.
Robert Bragg who now works at Embark Studios became the maintainer of this crate in October 2022.
In the process of working on the
jnicrate since becoming maintainer it's worth noting that I came across multiple APIs that I found needed to be re-worked to address safety issues, including ensuring that APIs that are not implemented safely are correctly declared asunsafe.There has been a focus on improving safety in the last two release.
The jni crate has been used in production with the Signal messaging application for over two years: https://github.com/signalapp/libsignal/blob/main/rust/bridge/jni/Cargo.toml
Some Notable Open Issues