* [NFC] Fix Type 4 Tag listener crashes on malformed NDEF write APDUs
While emulating a Type 4 Tag, type_4_tag_listener_iso_write() trusted the
reader-supplied Lc and NLEN, so four malformed UPDATE BINARY APDUs reached a
furi_check instead of an error status word:
00 D6 00 00 lc = 0 / data = NULL -> deref in bit_lib_bytes_to_num_be
00 D6 00 00 00 same, via the Le-only body
00 D6 00 00 01 AA lc -= write_len underflows size_t -> ~4 GB allocation
00 D6 00 00 02 FF FF NLEN never clamped -> ~128 KB transient allocation
Reject an APDU with no data field, clamp the NLEN bytes consumed to what the
reader actually sent, and bound the new length by ndef_max_len before it sizes
the array. The partial-NLEN write now merges into the current length the way
the read path already does, instead of zeroing the byte it did not cover.
Behaviour for a spec-compliant write (offset 0, Lc >= 2) is unchanged.
Closes#1050
* [NFC] Fix wrong parent protocol in Type 4 Tag listener assert
Copy-pasted from the SLIX listener: the Type 4 Tag listener sits on
ISO14443-4a, not ISO15693-3, so any debug build crashed as soon as Type 4 Tag
emulation received a frame. Release builds are unaffected (furi_assert is
compiled out).
* upd changelog
bit_lib_get_bits reads data[position/8 + 1] whenever position isn't
byte-aligned, even when all the requested bits live in the current byte.
When position/8 is the last byte of the buffer that's a one-byte
over-read. The extra byte only contributes bits that get shifted back
out, so the return value is unchanged and optimized builds often drop
the load, but at -O0 AddressSanitizer flags it and the access is still
out of bounds.
Skip the next-byte read when shift + length <= 8. This is the same fix
that already landed in fbtng-corelibs; lib/bit_lib here is an independent
copy that never picked it up, so the TODO FL-3534 comment is still here.
Signed-off-by: Cole Munz <colemunz@gmail.com>
eventLoop.timer(mode, interval) passes interval straight into
furi_ms_to_ticks() with no validation. For a periodic timer an interval
of 0 divides by zero in furi_event_loop_process_expired_timers()
(elapsed_time / timer->interval) and is counted as expired on every
event-loop pass, so a one-line script - eventLoop.timer("periodic", 0) -
pins the thread running the loop.
Reject interval <= 0 at the binding with a normal script-level error,
matching how the other js_app modules report bad arguments. The <= 0
check also covers negative values that would otherwise wrap to a huge
interval when cast to uint32_t.
Signed-off-by: Cole Munz <colemunz@gmail.com>
protocol_pyramid_add_wiegand_parity() computes the trailing odd-parity
bit with protocol_pyramid_get_parity(source + length / 2, ...). length is
a bit count (24), so source + length / 2 moves the pointer 12 bytes,
but wiegand[] in protocol_pyramid_encode() is 3 bytes. It should read
bits 12..23 of the same buffer.
So the parity bit we transmit comes from stack memory past wiegand[]
instead of the card number. Checked every 8-bit FC against every 16-bit
card number: the bit is wrong for essentially half of them, which a
reader that validates 26-bit Wiegand parity rejects. Our own decoder only
checks the CRC-8 and the format length, so reading a written card back
with a Flipper doesn't show it.
Give get_parity an explicit start position instead of doing pointer
arithmetic with a bit count. The even-parity call keeps reading bits
0..11 and is unchanged.
Signed-off-by: Cole Munz <colemunz@gmail.com>
protocol_pac_stanley_decode() fills an 8-byte asciiCardId[] and hands it
to hex_chars_to_uint8(), which takes no length and loops
while(*value_str && value_str[1]) - it stops at a terminator the buffer
doesn't have. So every successful decode reads asciiCardId[8] and [9] off
the end of the array, and if those two stack bytes happen to be hex
digits it also writes past protocol->data, which is 4 bytes.
Made the buffer 9 bytes and zeroed it. The loop still fills [0..7] and
the parse now stops after exactly 4 bytes, which is what
PAC_STANLEY_DECODED_DATA_SIZE expects.
Signed-off-by: Cole Munz <colemunz@gmail.com>
felica_poller_state_handler_list_system reads response_system_code->system_count
before checking whether felica_poller_list_system_code actually succeeded. That
function only writes the response pointer on success - on a timeout, a bad CRC,
or a response shorter than 13 bytes it returns an error and leaves the pointer
untouched. Since it's an uninitialized stack variable, every error path here
dereferences garbage.
This is the default path for Standard FeliCa cards, since auth is skipped by
default, so a weak read, a card pulled away mid-read, or an emulator that
doesn't answer Request System Code crashes the reader. That's the crash in
#4314.
Move the response handling inside the success branch, matching every other
handler in this file.
Signed-off-by: Cole Munz <colemunz@gmail.com>
infrared_common_decoder_alloc() sets protocol and level but leaves
timings_cnt uninitialized. Every caller allocs then resets, and for RC5
(the one protocol with preamble_mark == 0) the reset path reads
timings_cnt and feeds it to consume_samples() as a length before the
reset function zeroes it a few lines later. With a nonzero garbage value
that shifts the 6-element timings[] array out of bounds, both read and
write.
Separately, the bounds check in infrared_common_decode() compares
timings_cnt (an element count) against sizeof(decoder->timings) (24
bytes for six uint32_t), so it doesn't fire until 24 instead of 6.
Should be COUNT_OF.
Both come down to timings_cnt not being trustworthy; init it at alloc and
use the element count in the check.
Signed-off-by: Cole Munz <colemunz@gmail.com>
CodeQL runs are still reporting green, but every run carries a
failure-level annotation: "CodeQL Action major versions v1 and v2 have
been deprecated." It is running on a compatibility shim. Move all four
pins to v4, and bump checkout from v3 to v6 to match the rest of the
repo and clear the Node 20 deprecation warning. Drop
setup-python-dependencies, which the run log confirms has had no effect
since CodeQL 2.16. Add a concurrency group so back-to-back pushes to dev
no longer run several full firmware builds plus analyses at once.
pr-comment.yml trusted pr_number.txt from the report artifact and
validated it only as an integer. That artifact is written by a job that
checks out and runs the PR's own code, so both the comment body and its
destination were attacker-controlled: a malicious fork PR could make the
privileged commenter post arbitrary markdown onto any open thread in the
repo. Check the named PR's head SHA against the triggering run's
head_sha, which comes from the event payload and cannot be forged, and
refuse to comment on a mismatch.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Include documentation about caller thread context for GUI methods
* Add method for checking if view dispatcher id is already in use
* bump api
---------
Co-authored-by: Lofty Inclination <loftyinclination@outlook.com>
Co-authored-by: Stefan <66339601+turbospok@users.noreply.github.com>
* fix(subghz): park radio in boot state on Frequency Analyzer exit
The FA worker configures the internal CC1101 with a near-field AGC
profile (MDMCFG3/4, AGCCTRL0-2) and isolates the RF path, but exited
with only idle+sleep. SPWD retains configuration registers and the RF
switch lines keep their last state, so the radio parked with the
antenna disconnected and skewed AGC. Apps driving the radio HAL
directly (no reset / plain set_frequency) inherited a sticky
sensitivity loss until a full re-init; the built-in app and
subghz_devices-API apps rebuild state and were unaffected.
Reset the chip and re-park the RF switch on exit, matching the state
furi_hal_subghz_init establishes at boot. The explicit set_path is
needed because SRES reverts IOCFG2 to CHIP_RDYn, which deasserts in
sleep and would leave the switch half-selected on the 433 path.
Fixes#1044
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs: add changelog entry for Frequency Analyzer radio-state fix
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This reverts commit d71868364f.
The stale closed-thread backlog was locked silently beforehand, so the
restored daily lock-threads bot only comments on newly-idle threads
instead of re-flooding old ones.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* [NFC] Gate transit parser partial-read on the needed sector (#1042)
The transit parsers (plantain, troika, two_cities, sevppk_tk, szppk_so)
accepted any partial read as success. A parser read-success is terminal in
the read flow, so this skipped the dictionary/nested key-recovery tail even
when the parser's own data sector was never read.
Accept a partial read only when the sector parse() key-checks was actually
read (mf_classic_is_sector_read); otherwise report "not handled" so the app
runs the nested/dict-attack tail and re-parses.
Preserves the Plus 2K SL1 fix from #1038 (the needed low sectors are read on
those cards, so they stay accepted) and keeps genuine SL1/SL3 mixed-mode
cards on the parser path (their higher SL3 sectors are unrecoverable by
Crypto1 nested anyway).
Closes#1042
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(changelog): add PR #1043 to the transit parser fix entry
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* [NFC] Fix MIFARE Plus 2K SL1 transit parsers after MfClassicType2k
After #1016 promoted genuine MIFARE Plus S/X 2K cards in SL1 (SAK 0x08 +
Plus ATS) from MfClassicType1k to the new MfClassicType2k, the supported-
card parsers for those cards stopped working:
- Type gates (get_card_config / extract_purse_data) knew only 1K/4K, so
parse() rejected 2K -- plantain, troika.
- mf_classic_is_card_read() now needs all 32 sectors / 64 keys for a 2K
card, so a targeted read() that keys only its own sectors returns
MfClassicErrorPartialRead -> is_read = false -> the app diverts to the
dict-attack scene and never reaches parse() -- sevppk, szppk_so,
two_cities (and the above).
Handle MfClassicType2k explicitly as the Classic 1K these cards present
(same lower-sector keys / data sector), and accept a partial read in the
plugins' read() so parse() runs and validates the key.
social_moscow / kazan (plain Classic) and sk_tk (4K) are not promoted to
2K and are left unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(changelog): note Plus 2K SL1 transit parser fix (PR #1038)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* CCID: move USB layer from firmware HAL into ccid_test app
The CCID USB device interface no longer needs to live in the firmware.
A FAP can define its own FuriHalUsbInterface and drive the libusb_stm32
device stack directly: the usbd_* helpers are inline static in
usbd_core.h and dispatch through the dev->driver vtable, and those
headers already ship in the app SDK, so no new symbols must be exported.
- Move furi_hal_usb_ccid.{c,h} into applications/debug/ccid_test as
ccid_usb.{c,h} (interface renamed usb_ccid -> ccid_usb_interface,
furi_hal_usb_ccid_* -> ccid_usb_*). String-descriptor indices are
hardcoded since furi_hal_usb_i.h is not part of the SDK.
- Delete the firmware CCID module and its extern/include hooks.
- Drop the 4 CCID entries from f7 + f18 api_symbols.csv (keeping the
libusb usb_ccid.h header), bump API 87.3 -> 88.0 (breaking).
Frees ~1.7 KB of base-firmware flash; CCID code now only costs space
in the FAP while the app runs. Verified on hardware: enumerates as a
CCID reader (076b:3a21, class 0x0b) and passes the app's APDU tests.
* apply fbt format
---------
Co-authored-by: Štefan Croitoru <66339601+turbospok@users.noreply.github.com>
Daily dessant/lock-threads@v6 sweep + manual dispatch. Locks only closed
threads that have been inactive for 14 days; never closes anything and open
threads are untouched. Leaves a comment pointing users to open a fresh issue.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Run `fbt format` over the tree; normalizes whitespace, comment
spacing and argument wrapping in the bambu parser, Network/GPS RPC
services and ccid_test USB layer. No functional change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* [SubGHz] Fix crash when exiting CLI `subghz chat` with external CC1101
Pressing Ctrl+C to exit `subghz chat` while using an external CC1101
crashed the Flipper (null-pointer dereference + reboot).
The exit path called subghz_devices_deinit() before stopping the chat
worker. deinit unloads the external CC1101 radio-device plugin, freeing
its SubGhzDevice struct and interconnect vtable; the TxRx worker thread
sleeps/ends that device from its own thread on shutdown, so joining it
after deinit dereferenced freed plugin memory. The internal CC1101 is a
static device that deinit never frees, which is why the crash was
external-only.
Stop and free the worker before subghz_devices_deinit(), matching the
teardown order already used by subghz_cli_command_rx. Also add the
missing deinit + radio power-off to the "tx not allowed" early return so
a subsequent subghz command's subghz_devices_init() does not
furi_check-fail on an already-valid registry.
Fixes#829
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* changelog: SubGHz crash exiting CLI chat with external CC1101 (PR #1036, Fixes#829)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* [NFC] Data-drive MIFARE Plus manual-add generators
Replace the 18 near-identical MIFARE Plus generator thunks and their
handler-table entries with a data-driven mf_plus_generator_configs[]
table (uid_len / type / size / ATS per variant), dispatched via a small
range predicate. The Ultralight/NTAG and Classic types keep their
bespoke handlers, so the handler table now stops before the MF Plus
range.
Also drop mf_plus_ats_hist_ev, which was byte-identical to
mf_plus_ats_hist_s (EV1/EV2 are identified by GetVersion, not the ATS).
Pure refactor: the Add-Manually menu entries, the generated cards and the
public SDK API are all unchanged. Trims firmware flash by removing the
per-variant functions and the duplicate blob; a _Static_assert pins the
config table to the enum range so a future type can't desync it.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* [NFC] MIFARE Plus: single source for admin-key addresses
Fold the three copies of the admin-key address<->type mapping
(0x9000 CardMaster, 0x9001 CardConfig, 0x9003 L3Switch, 0x9004
SL1CardAuth) into one mf_plus_admin_key_addresses[] table with a forward
helper (mf_plus_get_admin_key_address, replaces the poller's local id
array) and a reverse helper (mf_plus_admin_key_type_from_address,
replaces two identical switch statements in the listener's key-resolve
and write-store paths).
Behavior-preserving: both listener default fall-throughs are kept (resolve
returns false; store falls through to data/config block handling), the
poller authenticates the same addresses, and a _Static_assert pins the
table to MfPlusAdminKeyNum. Internal helpers only; no SDK API change.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* [NFC] Changelog: MIFARE Plus generator/admin-map cleanup
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Mykhailo Shevchuk