Skip to content

fix(ios): Add SAFE_SHERPA_ONNX_OR macro to prevent FFI invalid pointe… #2682

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(ios): Add SAFE_SHERPA_ONNX_OR macro to prevent FFI invalid pointe… #2682

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 29 additions & 25 deletions sherpa-onnx/c-api/c-api.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1166,82 +1166,86 @@ struct SherpaOnnxOfflineTts {
std::unique_ptr<sherpa_onnx::OfflineTts> impl;
};

// iOS安全指针检查宏：防止FFI传递的无效指针导致崩溃
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

请问这个无效指针是哪里来的？

如果不为空，为什么还要额外判断？

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image image 这个是模型总结的问题，当然对我来说超纲了。 我对比两个版本APP是这样，修复后可以用，不修复的版本英文模型报错。

// 检查指针是否为null或指向低地址区域（通常是无效指针）
#define SAFE_SHERPA_ONNX_OR(x, y) ((x && (uintptr_t)(x) >= 0x1000) ? x : y)
Comment on lines +1169 to +1171
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Address the comment language and consider logging invalid pointers.

The macro implementation has several concerns:

  1. Comment language: The Chinese comment should be translated to English for consistency with the rest of the codebase.
  2. Threshold portability: The 0x1000 threshold is a heuristic that may not be reliable across all platforms or memory configurations. While common for detecting null-like low addresses, it's not guaranteed to catch all invalid pointers.
  3. Silent failures: When an invalid pointer is detected, the macro silently falls back to the default value without logging. This makes debugging difficult and hides the underlying issue.
  4. Root cause: This appears to be a workaround for Flutter FFI passing uninitialized pointers. Consider whether the FFI boundary should validate pointers before calling into native code.

Apply this diff to improve the implementation:

-// iOS安全指针检查宏：防止FFI传递的无效指针导致崩溃
-// 检查指针是否为null或指向低地址区域（通常是无效指针）
+// Safety macro for iOS: Prevents crashes from invalid pointers passed via FFI
+// Checks if pointer is null or points to low-address region (typically invalid pointers)
+// Note: 0x1000 threshold is a heuristic and may not catch all invalid pointers
-#define SAFE_SHERPA_ONNX_OR(x, y) ((x && (uintptr_t)(x) >= 0x1000) ? x : y)
+#define SAFE_SHERPA_ONNX_OR(x, y) \
+  (((x) && (uintptr_t)(x) >= 0x1000) ? (x) : \
+    (SHERPA_ONNX_LOGE("Invalid pointer detected at %s:%d, using fallback", __FILE__, __LINE__), (y)))

Additionally, consider whether this should be conditionally compiled for iOS only:

+#ifdef __APPLE__
 #define SAFE_SHERPA_ONNX_OR(x, y) ((x && (uintptr_t)(x) >= 0x1000) ? x : y)
+#else
+#define SAFE_SHERPA_ONNX_OR(x, y) SHERPA_ONNX_OR(x, y)
+#endif

🏁 Script executed:

#!/bin/bash
# Find macro definition and its comment
rg -n "SAFE_SHERPA_ONNX_OR" -C2 sherpa-onnx/c-api/c-api.cc

# Check other uses of the original macro
rg -n "SHERPA_ONNX_OR" -C2 sherpa-onnx/c-api/c-api.cc

# Inspect GetOfflineTtsConfig to see scope of SAFE_SHERPA_ONNX_OR usage
rg -n "GetOfflineTtsConfig" -C5 sherpa-onnx/c-api/c-api.cc

Length of output: 28746

Improve SAFE_SHERPA_ONNX_OR macro robustness

  • Translate the Chinese comments at lines 1169–1170 into English for consistency.
  • Enhance the macro (line 1171) to log when an invalid pointer is detected instead of silently falling back.
  • Guard the low-address heuristic (0x1000) behind an iOS-specific #ifdef __APPLE__ or clearly document its platform limitations.
  • Consider validating FFI pointers at the boundary rather than masking the root cause here.


static sherpa_onnx::OfflineTtsConfig GetOfflineTtsConfig(
const SherpaOnnxOfflineTtsConfig *config) {
sherpa_onnx::OfflineTtsConfig tts_config;

// vits
tts_config.model.vits.model = SHERPA_ONNX_OR(config->model.vits.model, "");
tts_config.model.vits.model = SAFE_SHERPA_ONNX_OR(config->model.vits.model, "");
tts_config.model.vits.lexicon =
SHERPA_ONNX_OR(config->model.vits.lexicon, "");
tts_config.model.vits.tokens = SHERPA_ONNX_OR(config->model.vits.tokens, "");
SAFE_SHERPA_ONNX_OR(config->model.vits.lexicon, "");
tts_config.model.vits.tokens = SAFE_SHERPA_ONNX_OR(config->model.vits.tokens, "");
tts_config.model.vits.data_dir =
SHERPA_ONNX_OR(config->model.vits.data_dir, "");
SAFE_SHERPA_ONNX_OR(config->model.vits.data_dir, "");
tts_config.model.vits.noise_scale =
SHERPA_ONNX_OR(config->model.vits.noise_scale, 0.667);
tts_config.model.vits.noise_scale_w =
SHERPA_ONNX_OR(config->model.vits.noise_scale_w, 0.8);
tts_config.model.vits.length_scale =
SHERPA_ONNX_OR(config->model.vits.length_scale, 1.0);
tts_config.model.vits.dict_dir =
SHERPA_ONNX_OR(config->model.vits.dict_dir, "");
SAFE_SHERPA_ONNX_OR(config->model.vits.dict_dir, "");

// matcha
tts_config.model.matcha.acoustic_model =
SHERPA_ONNX_OR(config->model.matcha.acoustic_model, "");
SAFE_SHERPA_ONNX_OR(config->model.matcha.acoustic_model, "");
tts_config.model.matcha.vocoder =
SHERPA_ONNX_OR(config->model.matcha.vocoder, "");
SAFE_SHERPA_ONNX_OR(config->model.matcha.vocoder, "");
tts_config.model.matcha.lexicon =
SHERPA_ONNX_OR(config->model.matcha.lexicon, "");
SAFE_SHERPA_ONNX_OR(config->model.matcha.lexicon, "");
tts_config.model.matcha.tokens =
SHERPA_ONNX_OR(config->model.matcha.tokens, "");
SAFE_SHERPA_ONNX_OR(config->model.matcha.tokens, "");
tts_config.model.matcha.data_dir =
SHERPA_ONNX_OR(config->model.matcha.data_dir, "");
SAFE_SHERPA_ONNX_OR(config->model.matcha.data_dir, "");
tts_config.model.matcha.noise_scale =
SHERPA_ONNX_OR(config->model.matcha.noise_scale, 0.667);
tts_config.model.matcha.length_scale =
SHERPA_ONNX_OR(config->model.matcha.length_scale, 1.0);
tts_config.model.matcha.dict_dir =
SHERPA_ONNX_OR(config->model.matcha.dict_dir, "");
SAFE_SHERPA_ONNX_OR(config->model.matcha.dict_dir, "");

// kokoro
tts_config.model.kokoro.model =
SHERPA_ONNX_OR(config->model.kokoro.model, "");
SAFE_SHERPA_ONNX_OR(config->model.kokoro.model, "");
tts_config.model.kokoro.voices =
SHERPA_ONNX_OR(config->model.kokoro.voices, "");
SAFE_SHERPA_ONNX_OR(config->model.kokoro.voices, "");
tts_config.model.kokoro.tokens =
SHERPA_ONNX_OR(config->model.kokoro.tokens, "");
SAFE_SHERPA_ONNX_OR(config->model.kokoro.tokens, "");
tts_config.model.kokoro.data_dir =
SHERPA_ONNX_OR(config->model.kokoro.data_dir, "");
SAFE_SHERPA_ONNX_OR(config->model.kokoro.data_dir, "");
tts_config.model.kokoro.length_scale =
SHERPA_ONNX_OR(config->model.kokoro.length_scale, 1.0);
tts_config.model.kokoro.dict_dir =
SHERPA_ONNX_OR(config->model.kokoro.dict_dir, "");
SAFE_SHERPA_ONNX_OR(config->model.kokoro.dict_dir, "");
tts_config.model.kokoro.lexicon =
SHERPA_ONNX_OR(config->model.kokoro.lexicon, "");
tts_config.model.kokoro.lang = SHERPA_ONNX_OR(config->model.kokoro.lang, "");
SAFE_SHERPA_ONNX_OR(config->model.kokoro.lexicon, "");
tts_config.model.kokoro.lang = SAFE_SHERPA_ONNX_OR(config->model.kokoro.lang, "");

// kitten
tts_config.model.kitten.model =
SHERPA_ONNX_OR(config->model.kitten.model, "");
SAFE_SHERPA_ONNX_OR(config->model.kitten.model, "");
tts_config.model.kitten.voices =
SHERPA_ONNX_OR(config->model.kitten.voices, "");
SAFE_SHERPA_ONNX_OR(config->model.kitten.voices, "");
tts_config.model.kitten.tokens =
SHERPA_ONNX_OR(config->model.kitten.tokens, "");
SAFE_SHERPA_ONNX_OR(config->model.kitten.tokens, "");
tts_config.model.kitten.data_dir =
SHERPA_ONNX_OR(config->model.kitten.data_dir, "");
SAFE_SHERPA_ONNX_OR(config->model.kitten.data_dir, "");
tts_config.model.kitten.length_scale =
SHERPA_ONNX_OR(config->model.kitten.length_scale, 1.0);

tts_config.model.num_threads = SHERPA_ONNX_OR(config->model.num_threads, 1);
tts_config.model.debug = config->model.debug;
tts_config.model.provider = SHERPA_ONNX_OR(config->model.provider, "cpu");
tts_config.model.provider = SAFE_SHERPA_ONNX_OR(config->model.provider, "cpu");
if (tts_config.model.provider.empty()) {
tts_config.model.provider = "cpu";
}

tts_config.rule_fsts = SHERPA_ONNX_OR(config->rule_fsts, "");
tts_config.rule_fars = SHERPA_ONNX_OR(config->rule_fars, "");
tts_config.rule_fsts = SAFE_SHERPA_ONNX_OR(config->rule_fsts, "");
tts_config.rule_fars = SAFE_SHERPA_ONNX_OR(config->rule_fars, "");
Comment on lines +1178 to +1248
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Verify that other config functions don't have the same vulnerability.

The SAFE_SHERPA_ONNX_OR macro is only applied to string fields in GetOfflineTtsConfig, but similar string pointer handling exists in other functions like GetOnlineRecognizerConfig, GetOfflineRecognizerConfig, GetKeywordSpotterConfig, etc. If Flutter FFI can pass invalid pointers to TTS config, it could potentially do the same for other configs.

Run the following script to identify other locations that may need similar protection:

Based on the results, consider either:

  1. Applying the same protection to other config functions, or
  2. Documenting why TTS config specifically requires this protection while others don't.

🏁 Script executed:

#!/bin/bash
# Description: Find all usages of SHERPA_ONNX_OR with string/pointer parameters in config functions

# Search for SHERPA_ONNX_OR calls with string-like parameters
rg -n 'SHERPA_ONNX_OR\(config->[^,]+\.(model|lexicon|tokens|data_dir|dict_dir|provider|encoder|decoder|vocoder|voices|lang|graph|rule_fsts|rule_fars|hotwords_file)[^)]*\)' --type cpp -C 2

echo "---"
echo "Summary: Review the above usages to determine if they should also use SAFE_SHERPA_ONNX_OR"

Length of output: 23186

Wrap all string‐pointer config fields with SAFE_SHERPA_ONNX_OR
Several config getters (recognizer, spotter, VAD, SLID, audio tagging, denoiser, punctuation, segmentation, embedding, etc.) currently use SHERPA_ONNX_OR directly on string/pointer fields (model, encoder, decoder, tokens, lexicon, etc.). Replace these with SAFE_SHERPA_ONNX_OR to guard against invalid pointers over FFI.

🤖 Prompt for AI Agents 
In sherpa-onnx/c-api/c-api.cc around lines 1178 to 1248, several config getters
use SHERPA_ONNX_OR directly on string/pointer fields which risks dereferencing
invalid FFI pointers; replace usages of SHERPA_ONNX_OR for any string or char*
fields (model, encoder, decoder, tokens, lexicon, voices, data_dir, dict_dir,
lang, etc.) with SAFE_SHERPA_ONNX_OR so they default to an empty string when the
pointer is invalid; leave numeric or boolean fields (noise_scale, length_scale,
num_threads, debug, etc.) using SHERPA_ONNX_OR as before and ensure resulting
tts_config fields are assigned the SAFE_* results.

tts_config.max_num_sentences = SHERPA_ONNX_OR(config->max_num_sentences, 1);
tts_config.silence_scale = SHERPA_ONNX_OR(config->silence_scale, 0.2);

Expand Down