Skip to content

ref: Address CWE-676 use of potentially dangerous functions#7549

Open
philprime wants to merge 2 commits intomainfrom
ref/cwe-676-dangerous-functions
Open

ref: Address CWE-676 use of potentially dangerous functions#7549
philprime wants to merge 2 commits intomainfrom
ref/cwe-676-dangerous-functions

Conversation

@philprime
Copy link
Copy Markdown
Member

@philprime philprime commented Feb 27, 2026

Address CWE-676 (use of potentially dangerous functions) reported by customer security tools. Replaces sscanf with strtoull/strtod in the report store and JSON codec so we avoid flagged APIs; scanf_s is not available on Apple platforms. Adds CWE-676 safety comments at all memcpy, strlen, and calloc sites documenting bounds or null-termination guarantees. Keeps calloc for zero-initialization and adds overflow guards where count*size could overflow (dirContents, image renderer). Adds unit tests for report ID parsing and floating-point decode.

Fixes #2785

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 27, 2026 •
edited
Loading

Semver Impact of This PR

🟢 Patch (bug fixes)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).

Internal Changes 🔧

Samples

  • Restructure iOS-Swift sample by philprime in #7654
  • Restructure samples and revert to sample-specific target names by philprime in #7659
  • Restructure iOS-Swift6 sample by philprime in #7656

Other

  • (deps) Update clang-format version by github-actions in #7675
  • Address CWE-676 use of potentially dangerous functions by philprime in #7549

🤖 This preview updates automatically when you update the PR.

@philprime philprime self-assigned this Feb 27, 2026
@philprime
Copy link
Copy Markdown
Member Author

philprime commented Feb 27, 2026

@sentry review

philprime
philprime commented Feb 27, 2026
View reviewed changes
Comment thread Sources/SentryCrash/Recording/Tools/SentryCrashJSONCodec.c
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 27, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 84.21053% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.170%. Comparing base (6baf12d) to head (7c4b659).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...ools/ViewCapture/SentryGraphicsImageRenderer.swift 75.000% 4 Missing ⚠️
...SentryCrash/Recording/Tools/SentryCrashFileUtils.c 33.333% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #7549       +/-   ##
=============================================
- Coverage   85.186%   85.170%   -0.016%     
=============================================
  Files          490       490               
  Lines        29520     29543       +23     
  Branches     12761     12774       +13     
=============================================
+ Hits         25147     25162       +15     
- Misses        4322      4331        +9     
+ Partials        51        50        -1
Files with missing lines Coverage Δ
Sources/Sentry/SentryAsyncSafeLog.c 100.000% <ø> (ø)
Sources/Sentry/SentrySessionReplaySyncC.c 67.241% <ø> (ø)
Sources/SentryCrash/Recording/SentryCrash.m 82.987% <ø> (ø)
...rces/SentryCrash/Recording/SentryCrashCachedData.c 84.800% <ø> (ø)
Sources/SentryCrash/Recording/SentryCrashReport.c 54.140% <ø> (ø)
...ces/SentryCrash/Recording/SentryCrashReportFixer.c 83.471% <100.000%> (ø)
...ces/SentryCrash/Recording/SentryCrashReportStore.c 81.889% <100.000%> (+0.898%) ⬆️
...Crash/Recording/Tools/SentryCrashCxaThrowSwapper.c 80.729% <ø> (ø)
...SentryCrash/Recording/Tools/SentryCrashJSONCodec.c 88.734% <100.000%> (ø)
...yCrash/Recording/Tools/SentryCrashMachineContext.c 79.487% <ø> (ø)
... and 4 more

... and 6 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6baf12d...7c4b659. Read the comment docs.

@philprime philprime added the ready-to-merge Use this label to trigger all PR workflows label Mar 13, 2026
@philprime
Copy link
Copy Markdown
Member Author

philprime commented Mar 13, 2026

@sentry review

@sentry
Copy link
Copy Markdown

sentry Bot commented Mar 13, 2026 •
edited
Loading

📲 Install Builds

iOS

🔗 App Name App ID Version Configuration
SDK-Size io.sentry.sample.SDK-Size 9.11.0 (1) Release

⚙️ sentry-cocoa Build Distribution Settings

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 13, 2026 •
edited
Loading

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1238.40 ms 1267.96 ms 29.55 ms
Size 24.14 KiB 1.15 MiB 1.13 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
787537a 1218.35 ms 1251.72 ms 33.38 ms
ffb6adc 1218.60 ms 1247.47 ms 28.87 ms
b6fa517 1218.83 ms 1257.47 ms 38.63 ms
df67624 1225.12 ms 1259.90 ms 34.78 ms
a7c42d9 1217.25 ms 1253.98 ms 36.73 ms
29d546e 1224.06 ms 1257.05 ms 32.98 ms
1c5ecda 1219.35 ms 1253.76 ms 34.41 ms
ae8cece 1216.83 ms 1251.37 ms 34.55 ms

App size

Revision Plain With Sentry Diff
787537a 24.14 KiB 1.15 MiB 1.12 MiB
ffb6adc 24.14 KiB 1.15 MiB 1.12 MiB
b6fa517 24.14 KiB 1.14 MiB 1.12 MiB
df67624 24.14 KiB 1.14 MiB 1.12 MiB
a7c42d9 24.14 KiB 1.15 MiB 1.13 MiB
29d546e 24.14 KiB 1.15 MiB 1.13 MiB
1c5ecda 24.14 KiB 1.15 MiB 1.12 MiB
ae8cece 24.14 KiB 1.15 MiB 1.13 MiB

Previous results on branch: ref/cwe-676-dangerous-functions

Startup times

Revision Plain With Sentry Diff
df27d25 1222.10 ms 1257.23 ms 35.13 ms

App size

Revision Plain With Sentry Diff
df27d25 24.14 KiB 1.12 MiB 1.10 MiB

@philprime
ref: Address CWE-676 use of potentially dangerous functions
c19551a 
Replace sscanf with strtoull/strtod in report store and JSON codec to
avoid CWE-676 flagged APIs on Apple platforms (scanf_s is unavailable).
Add CWE-676 safety comments at all memcpy, strlen, and calloc sites
documenting bounds or null-termination guarantees. Keep calloc for
zero-initialization; add overflow guards where count*size could
overflow (dirContents, image renderer). Add unit tests for report ID
parsing and floating-point decode.

Fixes #2785
@philprime philprime force-pushed the ref/cwe-676-dangerous-functions branch from eb59e2f to c19551a Compare April 29, 2026 09:17
@philprime
ref: Expand CWE-676 comments and add malformed-filename test
7c4b659 
Replace terse `// CWE-676: ...` annotations with full-sentence
justifications that explain why each strlen/memcpy/calloc call is safe
(or why the surrounding refactor avoided UB). Add a test that exercises
the production strncmp + strtoull filename parser via
sentrycrashcrs_getReportCount, ensuring malformed filenames in the
report directory are filtered out.
@philprime philprime marked this pull request as ready for review April 29, 2026 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@philipphofmann philipphofmann Awaiting requested review from philipphofmann philipphofmann is a code owner

@noahsmartin noahsmartin Awaiting requested review from noahsmartin noahsmartin is a code owner

@itaybre itaybre Awaiting requested review from itaybre itaybre is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@philprime philprime

Labels

ready-to-merge Use this label to trigger all PR workflows

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Use of potentially dangerous functions

1 participant

@philprime