Use of potentially dangerous functions

[philipphofmann]

Description

A customer reported that their security vulnerability tool reported our repository has the following security issue: CWE-676, which stands for the use of potentially dangerous functions.

For all tasks, we should check if we should do this quickly. If replacing is a bit complicated, needs refactoring to make things testable, we should reconsider the priority.

Clarified fixes for dangerous functions:

• Replace strncpy with strlcpy #2783
• Validate safe usage of return values of vsnprintf #2784
• ref: convert an sprintf call to snprintf #2866
• ref: convert last sprintf call to snprintf #3077

The following usage functions need clarification:

• memcpy - what are the downsides? Should we replace their usage?
• sscanf - Maybe replace it with safer scanf_s?
• strlen - you do not know the size of the original source buffer when using it. Is there a safer API?
• calloc - Should we replace our usages with malloc?

[github-actions]

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!

---

"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

[armcknight]

There is also a use of SHA1 in the crash reporter: https://github.com/getsentry/sentry-cocoa/blob/main/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_System.m#L424

[wkoutre]

👋🏼 Hi! I'm facing the same issue as the OP. I'm wondering if this has any priority. Thanks!

[philipphofmann]

@wkoutre, which warning do you get for which functions? We already fixed the most important functions. Fixing the rest in our backlog, but I can't give you an ETA.

[wkoutre]

@philipphofmann Thanks for the quick reply!

The warnings are for:

• Use of memcpy function
• Use of malloc function
• Use of SHA1

Another member of my team will follow up on this thread shortly with more details.

[juan-utility]

I update @wkoutre comment with the reported files:

Use of memcpy function
SentryCrashCString.m

Use of malloc function
SentryCrashCString.m

Use of SHA1
SentryDsn.m SentryCrashMonitor_System.m

Let me know if you need more information.

[philipphofmann]

@juan-utility and @wkoutre. We fix the occurrences in SentryCrashCString with #4045, and the use of SHA1 is something we can only change in the next major with #4022.

[brustolin]

Regarding SHA1. Its just a problem when used for security reason, which we dont used it for.

[wkoutre]

Thanks for the update here! We really appreciate it 🎉