Conversation
Enable stack protection to detect buffer overflows and harden the binary. This flag adds a canary to stack frames, causing the program to terminate immediately if stack corruption is detected. This helps identify memory safety bugs earlier during development by turning silent corruption into an immediate crash, and provides protection against stack-smashing exploits at runtime with negligible performance overhead. We don't add this generally because we wan't distributions/users to decide if they want it. But it can help us find problems early on during development.
Add security hardening via _FORTIFY_SOURCE=2 and switch to -Og to provide the necessary optimization for these checks while remaining debugger friendly. The goal is to catch many buffer overflows and format string errors at compile-time or runtime. -Og may occasionally optimize out very short-lived local variables, making them invisible in the debugger. So i'm not sure this is a very good idea. But I hope that we will find bugs earlier and don't even need to debug that often. We might revert this change later though in case we run into problems too often. I'm not aware of any other downsides.
Enable gccs static analyzer to detect memory bugs at compile-time. This validates our __attribute__((__cleanup__)) macros and improves security when parsing external XMPP data with zero runtime overhead.
It automatically aborts the program on allocation failure. Also eliminates the need for repetitive manual NULL checks.
Migrate structure allocations from calloc to glibs g_new0 macro to improve type safety and memory robustness. * Type Safety: The macro takes the type name directly, ensuring the allocated size always matches the pointer type. * Static Analysis: It guarantees a non-NULL return by aborting on failure, which silences -fanalyzer warnings regarding potential NULL pointer dereferences. * Readability: Removes redundant sizeof() calls and is the glib way
Make get_random_string() return a gchar*. This is part of the effort to migrate string declarations and allocations from char to gchar, replacing standard C allocators (malloc/calloc/strdup) with their glib equivalents (g_malloc/g_new0/g_strdup). The primary goal is to prevent mismatched allocator bugs. While char and gchar are binary compatible, mixing their respective deallocators is dangerous: * Freeing malloc'd memory with g_free() (or vice versa) can lead to memory corruption, double-frees, or crashes. * This is seems to be the case especiall on non Linux platforms or when using hardened memory allocators where the GLib slice allocator or wrappers may differ from the system heap. By standardizing on gchar, we ensure that our automatic cleanup macros (like auto_gchar) always invoke the correct deallocator (g_free).
This fixes a -fanalyzer uninitialized value warning.
Replace malloc() with g_malloc() to handle allocation failure and ensure enough memory is allocated for the null terminator.
* Cache strlen() result * Check for malloc() failure to prevent null pointer dereference * Ensure the output buffer is null-terminated for safe use with cons_show() * Simplify the quote escaping logic for better readability.
Modernize the /export command implementation by adopting GLib-based patterns for string manipulation and file handling. * Replace _writecsv with _append_csv_escaped to efficiently handle CSV quote doubling directly within a GString buffer * Refactor cmd_export to build the CSV content in memory using GString, minimizing system calls * Use g_file_set_contents for atomic writing
* Add NULL checks for malloc and strdup calls in cmd_sendfile * Ensure file handles and descriptors are closed on all error paths
And move whole preferences module to gchar.
That's not very nice yet but we have to start somewhere.
Usually we use `/help command`. Additionally there is a shortcut `/comamnd?`. 54fea4a tried to introduce to have `/command help`. Which doesn't really work since most commands get this as parameters. So it never worked. Additionally it introduced at segfault because it tried to set *question_mark to NULL even we can also get there without the `?` but by having the `help` as parameter.
recp elements were accessed without verifying the success of _ox_key_lookup(). Also fix several memory leaks. gpgme_data_new() followed 'gpgme_data_new_from_mem' was redundant.
Fix a potential NULL pointer dereference. And free the list properly.
data is allocated via g_new0() so cannot be NULL here. And it was used before the check already so that didn't make any sense in the first place.
Add NULL checks before calling 'strlen' and also ensures that the string is long enough before attempting to offset it by 3.
`userdata` was not freed if a handler could not be registered. This occurs when the 'id' parameter is NULL. In such cases, the handler cannot be stored in the 'id_handlers' table, but ownership of 'userdata' has already been transferred to this function. This commit ensures 'free_func' is called if 'id' is NULL since we always need to have an ID.
Move from popen() to g_spawn_command_line_sync(). Which is nicer and gets rid of a resource leak that was present here.
We were sequentially checking for errors from 'curl_easy_perform', 'ftell', and 'fclose'. Each time overwriting the last one, resulting in a leak. This commit ensures that 'err' is only set if it is currently NULL, preserving the first and most specific error encountered.
* _rotate_log_file(): guard against NULL mainlogfile before g_strdup/strlen * log_stderr_init(): move dup2 after malloc checks to avoid fd leak on allocation failure. Also remove explicit close(STDERR_FILENO) before dup2 since dup2 closes the target atomically. Close the fd returned by dup2 immediately.
Check for the memory before using it.
Member
Author
|
Good luck.. |
Not really a best practise. They should have been run before each commit and updated accordingly. Next time..
jubalh
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Let's build with -fanalyzer in debug mode.
These commits are related to issues found by gccs static analyzer.
Also bulid with -fstack-protector-strong compiler flag.
This flag adds a canary to stack frames, causing the program to terminate immediately if stack corruption is detected.
This helps identify memory safety bugs earlier during development by
turning silent corruption into an immediate crash.