[GH-ISSUE #2487] [Store][TE] Three signed-char `::tolower`/`::toupper` UB sites missed by #2367 #662

New issue

Closed

opened 2026-10-29 21:17:47 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-29 21:17:47 +00:00

Owner

Originally created by @bp-cheng on GitHub (Jun 15, 2026).
Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/2487

Originally assigned to: @bp-cheng on GitHub.

PR #2367 ("[Common][Store][TE] Fix undefined behavior passing signed char to ::tolower") fixed four std::transform(..., ::tolower) sites where a (possibly signed) char is passed to ::tolower/::toupper. A byte > 0x7F sign-extends to a negative int, which is UB for these functions (the argument must be representable as unsigned char or equal EOF).

Three sibling occurrences on operator/CLI-controlled input were missed and are still present on current main (9afefdb1):

mooncake-transfer-engine/src/transport/tcp_transport/tcp_transport.cpp:606 — std::transform(val.begin(), val.end(), val.begin(), ::tolower); on the value of env var MC_TCP_ENABLE_CONNECTION_POOL. (<cctype> is only available transitively here.)
mooncake-store/include/utils.h:225 — std::transform(unit.begin(), unit.end(), unit.begin(), ::toupper); on the unit suffix of a size string, fed by env MC_MMAP_ARENA_POOL_SIZE, the global_segment_size CLI flag, and config-map values. Note line 274 of this same file already uses the safe lambda idiom — line 225 was simply overlooked.
mooncake-store/include/ha/oplog/oplog_store_factory.h:34 — std::transform(lower.begin(), lower.end(), lower.begin(), ::tolower); on the HA oplog_store_type config string. (<cctype> only transitively available.)

All three take operator/attacker-controllable bytes (env vars, CLI flags, HA config), not kernel-generated ASCII like PCI bus IDs. Practical crash risk on glibc is low (its ctype tables span -128..255), but this is standard-defined UB that the project already chose to fix in #2367; these are the remaining sites that complete that sweep.

Will send a PR applying the same [](unsigned char c) { return std::tolower(c); } idiom, adding <cctype> where it is only transitively available.

Originally created by @bp-cheng on GitHub (Jun 15, 2026). Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/2487 Originally assigned to: @bp-cheng on GitHub. PR #2367 ("[Common][Store][TE] Fix undefined behavior passing signed char to `::tolower`") fixed four `std::transform(..., ::tolower)` sites where a (possibly signed) `char` is passed to `::tolower`/`::toupper`. A byte > 0x7F sign-extends to a negative `int`, which is UB for these functions (the argument must be representable as `unsigned char` or equal `EOF`). Three sibling occurrences on operator/CLI-controlled input were missed and are still present on current main (`9afefdb1`): 1. `mooncake-transfer-engine/src/transport/tcp_transport/tcp_transport.cpp:606` — `std::transform(val.begin(), val.end(), val.begin(), ::tolower);` on the value of env var `MC_TCP_ENABLE_CONNECTION_POOL`. (`<cctype>` is only available transitively here.) 2. `mooncake-store/include/utils.h:225` — `std::transform(unit.begin(), unit.end(), unit.begin(), ::toupper);` on the unit suffix of a size string, fed by env `MC_MMAP_ARENA_POOL_SIZE`, the `global_segment_size` CLI flag, and config-map values. Note line 274 of this same file already uses the safe lambda idiom — line 225 was simply overlooked. 3. `mooncake-store/include/ha/oplog/oplog_store_factory.h:34` — `std::transform(lower.begin(), lower.end(), lower.begin(), ::tolower);` on the HA `oplog_store_type` config string. (`<cctype>` only transitively available.) All three take operator/attacker-controllable bytes (env vars, CLI flags, HA config), not kernel-generated ASCII like PCI bus IDs. Practical crash risk on glibc is low (its ctype tables span -128..255), but this is standard-defined UB that the project already chose to fix in #2367; these are the remaining sites that complete that sweep. Will send a PR applying the same `[](unsigned char c) { return std::tolower(c); }` idiom, adding `<cctype>` where it is only transitively available.

bsakaguchi closed this issue

2026-10-29 21:17:47 +00:00

bsakaguchi commented

2026-10-29 21:17:48 +00:00

Author

Owner

@github-actions[bot] commented on GitHub (Jun 15, 2026):

Thanks for opening this issue, @bp-cheng!

Field	Value
Issue	#2487
GitHub user ID	`194346132`
Reporter	@bp-cheng

A maintainer will triage this when possible. To help us respond faster, please include:

Mooncake version or commit SHA
Environment (OS, CUDA/driver, RDMA stack if relevant)
Steps to reproduce and expected vs. actual behavior

Useful links: Documentation · Contributing guide

This message was posted automatically by the issue bot.

@github-actions[bot] commented on GitHub (Jun 15, 2026): Thanks for opening this issue, @bp-cheng! | Field | Value | |-------|-------| | **Issue** | #2487 | | **GitHub user ID** | `194346132` | | **Reporter** | @bp-cheng | A maintainer will triage this when possible. To help us respond faster, please include: - Mooncake version or commit SHA - Environment (OS, CUDA/driver, RDMA stack if relevant) - Steps to reproduce and expected vs. actual behavior Useful links: [Documentation](https://kvcache-ai.github.io/Mooncake/) · [Contributing guide](https://github.com/kvcache-ai/Mooncake/blob/main/CONTRIBUTING.md) > This message was posted automatically by the issue bot.

bsakaguchi referenced this issue

2026-10-29 21:29:50 +00:00

[PR #662] [MERGED] [DOC] add SGLang RDMA trouble shooting #1726