Throw exception rather than SIGSEGV by 10gic · Pull Request #4443 · trustwallet/wallet-core

10gic · 2025-07-05T14:29:49Z

Description

Fix issue #4442

How to test

Types of changes

Bug fix (non-breaking change)

Checklist

Create pull request as draft initially, unless its complete.
Add tests to cover changes as needed.
Update documentation as needed.
If there is a related Issue, mention it in the description.

10gic · 2025-07-08T02:57:46Z

Hi @satoshiotomakan, could you please take some time to review it?

delaram7

@10gic
کمک کن لطفا به من

10gic · 2025-08-22T02:35:47Z

Hi @satoshiotomakan, just a friendly bump on the PR when you have a moment!

10gic · 2025-09-08T09:51:09Z

Hi @satoshiotomakan, could you give this PR a quick review when you have time? Appreciate it!

10gic · 2025-09-12T05:59:52Z

My JVM instance occasionally produces core dumps due to this issue. The Java server is complex, so I can’t check every parameter in the caller. I really need this PR to be merged. Could you please have a look at this PR? @satoshiotomakan

satoshiotomakan · 2025-09-12T13:29:55Z

Hi @10gic, sorry for the late answer, let me take a look

satoshiotomakan

Only one question please

satoshiotomakan · 2025-09-12T13:41:47Z

jni/cpp/TWJNI.h

+#define JNI_CHECK_NULL_AND_RETURN_VOID(env, param, paramName) \
+    do { \
+        if (param == NULL) { \
+            jclass exceptionClass = (*env)->FindClass(env, "java/lang/IllegalArgumentException"); \


Should we check if exceptionClass is not null like

if (exceptionClass == NULL) { return throwNoClassDefError(env, "java/lang/IllegalArgumentException"); }

@satoshiotomakan You are always so careful. Normally, the class java/lang/IllegalArgumentException should always be available. However, adding extra checks is a practice of defensive programming.
BTW, if java/lang/IllegalArgumentException is not available, it would be difficult to properly define the throwNoClassDefError function, so I simply return early instead.

satoshiotomakan · 2025-09-12T13:44:02Z

@gupnik can you please also take a look?

gupnik

LGTM. Thanks!

satoshiotomakan

LGTM!

* fiux(ci): Use `iPhone 16` in ios-release (#4493) * Throw exception rather than SIGSEGV (#4443) * Throw exception rather than SIGSEGV * Fix test cases * Add NULL checker for exceptionClass * feat(Plasma): Add Plasma Mainnet (#4499) * feat(plasma): Add Plasma Mainnet * feat(plasma): Add mobile tests * [Solana]: Adds ability to transfer tokens to the feepayer (#4503) * Initial setup * Minor updates * Restructure * fix(barz): Replace u32 parameters with i32 (#4504) * Disallow to use unsigned integer parameters within class methods * fix(data): Allocate empty array on `tw_data_create_with_bytes` if data is NULL (#4508) * fix(data): Allocate empty array on `tw_data_create_with_bytes` if data is NULL * fix(data): Add TWData test * feat(Biz): Add helper functions to support `BizPasskeySessionAccount` (#4516) * feat(biz-passkey): Add `TWBarzEncodeRegisterSessionCall` FFI * feat(biz-passkey): Add `TWBarzEncodeRemoveSessionCall` FFI * feat(biz-passkey): Add `TWBarzEncodePasskeySessionNonce` FFI * feat(biz-passkey): Add `TWBarzEncodeExecuteWithPasskeySessionCall` FFI * refactor(barz): Move some functions to TWBiz and TWEip7702 modules * feat(biz): Add WebAuthn * feat(biz): Add `TWWebAuthnGetMessageHash` and `TWWebAuthnGetFormattedSignature` * feat(biz): Fix C++ and Mobile * Rename `TWWebAuthn` module to `TWWebAuthnSolidity` * feat(biz): Adjust `executeWithPasskeySession` arguments * feat(biz): Fix fmt * feat(biz): Add Biz Android tests * feat(biz): Add final Biz Android test * feat(biz): Fix lints * feat(biz): Minor change * feat(biz): fmt * feat(solana): Add `SolanaTransaction.insertTransferInstruction()` (#4523) * feat(solana): Add `transfer_to_fee_payer` instruction * feat(solana): Add `tw_solana_transaction_insert_transfer_instruction` FFI * feat(solana): Add a note comment * feat(solana): Add one more comment * feat(zcash): Add support for TEX address (#4527) * feat(zcash): Add `TexAddress` in Rust * feat(zcash): Add successfully broadcasted transaction test * feat(zcash): Add successfully broadcasted transaction C++ test * feat(zcash): Fix C++ tests * feat(zcash): Reuse code in `TexAddress::isValid` * feat(zcash): Fix `TexAddress:isValid` * [Solana]: Renames fee recipient token account for clarity (#4528) * [Solana]: Renames fee recipient token account for clarity * Fix typo * Feature: Add trade and secured fields to THORChain Asset (#4500) * my tracked commit * MsgInstantiateContract * revert formatting * test * Tests fixes and broadcast * cargo fmt * remove unused * feat: add trade and secured fields to Thorchain Asset - Add trade and secured boolean fields to Asset proto message - Update ThorchainAsset struct in Rust with new fields - Update asset initialization in tx_builder.rs - Matches Thorchain blockchain Asset struct definition --------- Co-authored-by: gupnik <mail.guptanikhil@gmail.com> * feat(kusama-asset-hub): Add ability to force to use `ChargeAssetTxPayment` (#4541) * feat(kusama-asset-hub): Add ability to force to use `ChargeAssetTxPayment` for native tip * feat(kusama-asset-hub): Add a unit test * feat(monad): Add Monad testnet (#4543) * feat(monad): Update Monad to mainnet (#4550) * TODO update explorer * feat(monad): Update block explorer (#4553) * feat(monad): Update explorer * feat(monad): Update explorer schema * Fix incorrect value of NO_PAD constant (#4487) Co-authored-by: Sergei Boiko <127754187+satoshiotomakan@users.noreply.github.com> * fix(public-key): Fix `PublicKey::verify` by adding signature length validation (#4565) * fix(public-key): Fix `PublicKey::verify` by adding signature length validation * fix(public-key): Fix PR comments * Fix Reveal Operations for Seoul Protocol (#4557) * Since the Seoul protocol, reveal operations require the presence of a boolean for whether or not the proof field is present. This is required when manually forging bytes. Added. * Fix TezosTests.swift * Fix TezosTests.swift * Update tests * Update tests * Fix TezosCompiler.CompileWithSignatures * Fix TezosCompiler.CompileWithSignatures * fix(tezos): Fix Tezos compile test * fix(tezos): Fix Tezos signing test * fix(tezos): Add mainnet test transaction --------- Co-authored-by: gupnik <nikhil.g@trustwallet.com> Co-authored-by: Sergei Boiko <satoshiotomakan8@gmail.com> * fix(public-key): Check zilliqa schnorr signature size (#4571) * fix(public-key): Check zilliqa schnorr signature size * Replace assert macros with proper checks * fix(public-key): Address a PR comment * fix(public-key): Address a PR comments * fix(public-key): Fix a bug in PublicKey constructor * feat(pczt): Add implementation of PCZT signing (#4576) * feat(pczt): Add implementation of PCZT signing * feat(pczt): Add a successfully broadcasted tx test * feat(pczt): Address PR comments * fix(webauthn): Fix buffer out of bounds (#4577) * fix(webauthn): Fix buffer out of bounds * fix(webauthn): Fix `TWWebAuthnGetRSValues` implicit Data conversion * fix(webauthn): Extra checks for algorithm, and public key parts * fix(webauthn): Test common buffer overflows * fix(webauthn): Check `crv` and `kty` parameters * fix(private-key): Add PrivateKey Data move constructor * chore(noexcept): Remove wrong noexcept * fix(private-key): Zeroize memory on move `operator=` * fix(private-key): Avoid cleaning private key if self-assign * fix(aes): Add iv size validation (#4580) * fix(aes): Add iv size validation * fix(aes): Add unit tests * Update Dockerfile to install cbindgen with --locked and provide explicit links to clang binaries (#4568) * install cbindgen with --locked * add explicit links to clang/clang++ since CMake will not use the ENV --------- Co-authored-by: Sergei Boiko <127754187+satoshiotomakan@users.noreply.github.com> * feat(stable-account): Add `TWDerivationSmartChainStableAccount` derivation type (#4585) * fix(overflow): Fix integer overflows in `BinaryEncoding` and `Pactus` (#4592) * fix(binary-coding): Avoid unsigned integer overflow * fix(pactus): Avoid unsigned integer overflow * feat(tron): Add `raw_data_hex` parameter in JSON transaction representation (#4593) * feat(tron): Add `raw_data_hex` parameter in JSON transaction representation * feat(tron): Fix iOS test * feat(biz): Add `Biz.signExecuteWithSignatureCall` and `BizPasskeySession.signExecuteWithSignatureCall` (#4594) * feat(biz): Add `Biz.SignExecuteWithSignatureCall` * Add new `BizPasskeySession` module * feat(biz): Add `Biz.SignExecuteWithSignatureCall` * Finish `BizPasskeySession` module separation * feat(biz): Fix android tests * feat(biz): Address PR review comments * chore(codeowners): Add onchain-corex-enabling team to CODEOWNERS (#4600) * fix(ethereum-abi): Fix Ethereum ABI type parsing recursion + chore actions and toolchain (#4597) * fix(ethereum-abi): Fix Ethereum ABI type parsing recursion * fix(ethereum-abi): Freeze community github actions * chore(rust): Bump rust toolchain version to nightly-2025-12-11 * chore(wasm): Try to update emsdk to 4.0.22 * chore(rust): Decrease code coverage to 94.1 due to a recent toolchain update * chore(rust): Freeze `actions` hashes --------- Co-authored-by: Sergei Boiko <satoshiotomakan8@gmail.com> Co-authored-by: Sergei <> * chore(actions): Replace unverified actions with GH scripts (#4604) * chore(actions): Replace unverified actions with GH scripts * chore(actions): Update rust toolchain in Dockerfile * fix(bitcoin-v2): Fail if `max_amount_output` provided with `outputs` or `change_output` at a time (#4607) * fix(bitcoin-v2): Fail if `max_amount_output` provided with `outputs` or `change_output` at a time * fix(bitcoin-v2): PR notes * fix(cbor): Fix Cbor Map with odd elements number decoding (#4610) * fix(cbor): Fix Cbor Map with odd elements number decoding * fix(cbor): Change test name * chore(sync): Fix merge conflicts Breaking changes: * The following FFIs take i32 instead of u32 due to Kotlin bindings limitations: `tw_hd_node_create_with_seed`, `tw_hd_node_create_with_extended_private_key`, `tw_hd_node_derive_from_path`, `tw_hd_node_extended_private_key`, `tw_hd_node_extended_public_key`, `tw_hd_node_public_create_with_extended_public_key`, `tw_hd_node_public_derive_from_path`, `tw_mnemonic_generate`, `tw_mnemonic_get_word`, `tw_pbkdf2_hmac_sha512`, `tw_ecdsa_pubkey_hash` * `crypto_scrypt` FFI is no longer marked as `tw_ffi` to avoid changing u32 and usize argument types to i32 * chore(sync): Increase test coverage * Add Bitcoin PSBT compile test * Add `PrivateKey.CopyAssignmentOperator` and `PrivateKey.MoveAssignmentOperator` tests * Copy Rust TWMnemonic tests to C++ * Copy Rust Biz tests to C++ * Copy Rust BizPasskeySession tests to C++ * chore(sync): Push missing PSBT implementation + remove unused code * chore(sync): Fix Biz android tests --------- Co-authored-by: Sergei Boiko <127754187+satoshiotomakan@users.noreply.github.com> Co-authored-by: 10gic <github10gic@proton.me> Co-authored-by: gupnik <nikhil.g@trustwallet.com> Co-authored-by: Enrique Souza <enriquesouza@live.com> Co-authored-by: gupnik <mail.guptanikhil@gmail.com> Co-authored-by: Corey Soreff <csoreff@gmail.com> Co-authored-by: Sergei Boiko <satoshiotomakan8@gmail.com> Co-authored-by: Joey Yandle <xoloki@gmail.com>

* Throw exception rather than SIGSEGV * Fix test cases * Add NULL checker for exceptionClass

10gic requested review from gupnik, rkokhatskyi and satoshiotomakan as code owners July 5, 2025 14:29

10gic force-pushed the throw-exception-for-null-parameter branch from 60ffd7a to bfa8d52 Compare August 4, 2025 04:36

delaram7 reviewed Aug 4, 2025

View reviewed changes

10gic force-pushed the throw-exception-for-null-parameter branch from bfa8d52 to 0f21679 Compare August 5, 2025 04:26

10gic added 2 commits September 11, 2025 10:44

Throw exception rather than SIGSEGV

3230ce7

Fix test cases

4dac1fc

10gic force-pushed the throw-exception-for-null-parameter branch from 8aa97ed to 4dac1fc Compare September 11, 2025 02:44

satoshiotomakan reviewed Sep 12, 2025

View reviewed changes

Add NULL checker for exceptionClass

dc0932d

gupnik approved these changes Sep 15, 2025

View reviewed changes

satoshiotomakan approved these changes Sep 16, 2025

View reviewed changes

satoshiotomakan merged commit 3c1f86c into trustwallet:master Sep 16, 2025
15 checks passed

10gic mentioned this pull request Sep 17, 2025

Null value cause SIGSEGV in JVM #4442

Closed

perpetua-engineering pushed a commit to perpetua-engineering/wallet-core that referenced this pull request Feb 25, 2026

Throw exception rather than SIGSEGV (trustwallet#4443)

55b5b85

* Throw exception rather than SIGSEGV * Fix test cases * Add NULL checker for exceptionClass

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Throw exception rather than SIGSEGV#4443

Throw exception rather than SIGSEGV#4443
satoshiotomakan merged 3 commits intotrustwallet:masterfrom
10gic:throw-exception-for-null-parameter

10gic commented Jul 5, 2025

Uh oh!

10gic commented Jul 8, 2025

Uh oh!

delaram7 left a comment

Uh oh!

10gic commented Aug 22, 2025

Uh oh!

10gic commented Sep 8, 2025

Uh oh!

10gic commented Sep 12, 2025

Uh oh!

satoshiotomakan commented Sep 12, 2025

Uh oh!

satoshiotomakan left a comment

Uh oh!

satoshiotomakan Sep 12, 2025

Uh oh!

10gic Sep 12, 2025

Uh oh!

satoshiotomakan commented Sep 12, 2025

Uh oh!

gupnik left a comment

Uh oh!

satoshiotomakan left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

10gic commented Jul 5, 2025

Description

How to test

Types of changes

Checklist

Uh oh!

10gic commented Jul 8, 2025

Uh oh!

delaram7 left a comment

Choose a reason for hiding this comment

Uh oh!

10gic commented Aug 22, 2025

Uh oh!

10gic commented Sep 8, 2025

Uh oh!

10gic commented Sep 12, 2025

Uh oh!

satoshiotomakan commented Sep 12, 2025

Uh oh!

satoshiotomakan left a comment

Choose a reason for hiding this comment

Uh oh!

satoshiotomakan Sep 12, 2025

Choose a reason for hiding this comment

Uh oh!

10gic Sep 12, 2025

Choose a reason for hiding this comment

Uh oh!

satoshiotomakan commented Sep 12, 2025

Uh oh!

gupnik left a comment

Choose a reason for hiding this comment

Uh oh!

satoshiotomakan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants