Repository navigation
[Unsafe] Guard JNI string lengths - #13043
Merged
Merged
Conversation
`Android.Runtime.JNIEnv.NewString()` forwards its length to JNI without a bounds check. Reject negative and oversized lengths before pinning while preserving the existing null-first return behavior. Context: #11467 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Empty arrays with zero length still pass a null pointer to JNI NewString.
1 open finding
What changed in this PR
Adds bounds validation to safely marshal character-array prefixes into JNI strings.
Changes:
- Rejects negative and oversized lengths.
- Adds on-device regression coverage for bounds and prefix marshaling.
| File | Description |
|---|---|
src/Mono.Android/Android.Runtime/JNIEnv.cs |
Validates JNI string lengths. |
tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JnienvTest.cs |
Tests invalid lengths and valid prefixes. |
🧠 Review effort: Balanced
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
simonrozsival
changed the title
simonrozsival
added
the
ready-to-review
Member
Author
|
@dalexsoto review |
simonrozsival
enabled auto-merge (squash)
October 9, 2026 12:52
dalexsoto
approved these changes
Oct 9, 2026
dalexsoto
left a comment
dalexsoto
left a comment
Member
There was a problem hiding this comment.
Invalid array lengths are rejected before pinning or JNI access, while null-first compatibility and valid prefix marshaling are preserved. The zero-length path now uses the existing empty-string helper safely. The complete correctness and integration review leaves no blocking issue.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
title and
description
should follow the
commit-messages.mdworkflow documentation, and in particular should include:JNIEnv.NewString(char[]?, int)previously forwarded negative or oversized lengths to JNI without checking the source array. It now rejects invalid bounds before pinning/native access, retains the existing null-first behavior, and still accepts legal prefixes. One regression test covers negative and oversized lengths, null with an invalid length, and valid prefix marshaling.Context: #11467 (related background only; remains open).
Validation
make prepare && make all— failed during bootstrap restore because the machine-wide SDK requested unavailableMicrosoft.NETCore.App.Ref10.0.13.PATH="$PWD/bin/Debug/dotnet:$PATH" make prepare && PATH="$PWD/bin/Debug/dotnet:$PATH" make all— prepare and solution compilation passed; final workload configuration failed because the API 37.1 reference assembly was missing.PATH="$PWD/bin/Debug/dotnet:$PATH" make leeroy— passed, including the extra API-level build and local workload configuration../dotnet-local.sh build -t:Install -c Debug -p:IncludeCategories=JniStringLength -p:Device=emulator-5570 -p:AdbTarget="-s emulator-5570" tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj— passed with 0 warnings and 0 errors; emulator only.(cd tests/Mono.Android-Tests/Mono.Android-Tests && ../../../dotnet-local.sh test Mono.Android.NET-Tests.csproj --no-build --device emulator-5570 -c Debug -p:IncludeCategories=JniStringLength -p:AdbTarget="-s emulator-5570" --report-trx --results-directory ../../../bin/TestDebug/TestResults)— passed, 1/1 regression test onemulator-5570.The full on-device runtime suite was not run. No physical device was used.