Security hardening: glucose ingestion, sensor isolation, outbound broadcasts - #89
Open
JetFoxy wants to merge 5 commits into
Open
Security hardening: glucose ingestion, sensor isolation, outbound broadcasts#89JetFoxy wants to merge 5 commits into
JetFoxy wants to merge 5 commits into
Conversation
RE of BLE protocol from AndroidAPS medtrum module + APK analysis. Documents BLE UUIDs, packet framing, auth flow, CGM notification format (MASK 0x1000, 5 bytes), and JugglucoNG driver architecture. CGM field byte layout marked as hypothesis pending BLE HCI validation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add libmdkjnidemo.so wrapper (JniUtil JNI stub + MedtrumNativeLoader) following BlecommLoader pattern. Used for glucose prediction to match official app output. Both arm64-v8a and armeabi-v7a variants listed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
M4 (P1): Document broadcast package-squatting finding as OPEN in review doc. The getPackageInfo() checks reduce unnecessary broadcasts but do not authenticate the receiving app. Full signature pinning deferred until known-good certificate fingerprints are available for companion apps. M5 (P1): Native bounds guard in addGlucoseInjection already present (g.cpp:1222). Consistent with addGlucoseStreamInternal — rejects NaN/Inf/negative/>1200. M6 (P2): Group-by-sensor smoothing already implemented. GlucosePoint.sensorSerial + DataSmoothing.smoothSensorGroups() prevent cross-sensor averaging contamination. M7 (P2): Expired sensor filtering already implemented. ManagedSensorIdentityAdapter.isExpired() + SensorBluetooth candidate filtering before sort in resolvePreferredCurrentSensor. M8 (P3): Add 35 comprehensive tests pinning all JSON field aliases (primary/auto/raw mg/dL and mmol variants), negative tests for zero/negative/NaN/Inf/garbage/missing-timestamp values, and rate bounds verification. All security-related tests pass (138 total, 4 pre-existing failures in unrelated classes).
Broadcasts.updateall() queried packages listening for glucodata.Minute (GlucoDataHandler) but stored the result via seteverSenseRecepters instead of setglucodataRecepters. Users migrating from initVersion<13 with GDH broadcast already enabled silently stopped receiving glucose data in GDH after upgrading, while EverSense's receiver list got overwritten with the wrong packages.
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
1 participant
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.
Summary
Security hardening for the glucose data-ingestion, sensor-isolation, and
outbound-broadcast paths, following an internal security edge-case review
and a subsequent correctness re-check of that review's own fixes.
(
VirtualGlucoseSensorBridge,NightscoutFollowerManager,g.cpp).ManagedSensorIdentityRegistrythat couldwipe a sensor's view-mode/identity cache while another adapter still
legitimately held it.
in-app message instead of a silent scheme rewrite.
an earlier pass of this review had accidentally dropped (would have
silently stopped ingesting readings for sources using those key names).
(Wear, Gadgetbridge, XInfuus/LibreLink, xDrip-compatible, EverSense).
sensorSerialand filter expired sensors fromcandidate resolution, closing two items the original review had to defer.
registry, setup wizard) that was in flight alongside this hardening work.
Full review notes and the prioritized follow-up plan (including one item,
signature-based package verification for broadcasts, that remains
intentionally open) are included at
.omo/reviews/security-edge-case-review.mdand
.omo/plans/juggluco-security-edge-review-followup.md.Known follow-up (not in this PR)
that it's the genuine signed app — package-name squatting is not fully
closed. Tracked as open in the review doc; needs signing-certificate
pinning as separate follow-up work.
Test plan
ApiGlucoseSourceSecurityTestscovering all JSON field aliases(positive) and out-of-range/garbage values (negative).
DataSmoothingTests,NightscoutFollowerIntegrationTests,NightscoutFollowerRegistryTestsfor the new bounds/grouping logic../gradlewunit test run (could not be executed in thisenvironment — only JDK 16 available, project requires JDK 17).
entry shows the new in-app warning, broadcast targets only fire to
installed companion apps.