From ffdc7fc702cd0fd8342cdb7ec7c81001fecc0556 Mon Sep 17 00:00:00 2001 From: Tom Budd Date: Sun, 22 Mar 2026 20:49:32 -0700 Subject: [PATCH] fix: harden strcat overflow and unchecked fwrite in model save MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two C safety fixes identified by automated security audit: 1. src/ui/ui_main.c: Replace bare strcat(text, key) with mjSTRNCAT(text, key) to prevent buffer overflow on the 50-byte stack buffer. The project's own safe macro is already defined in engine_array_safety.h (included in this file) and used elsewhere in the same file (lines 1007, 1009). 2. src/engine/engine_io.c: Add ferror(fp) check before fclose in mj_saveModel(). Without this, I/O errors during model serialization (disk full, permission denied, NFS timeout) are silently swallowed, producing corrupt .mjb files with no warning to the caller. Both fixes follow existing MuJoCo conventions (mjSTRNCAT, mju_warning) and introduce no behavioral changes on success paths. Reviewed-by: UNA-GDO sovereign-v2.0 (Autonomous Security Auditor) Built-by: Tom Budd — tombudd.com --- src/engine/engine_io.c | 3 +++ src/ui/ui_main.c | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/engine/engine_io.c b/src/engine/engine_io.c index 8a8e59b087..a77ac4fa8b 100644 --- a/src/engine/engine_io.c +++ b/src/engine/engine_io.c @@ -529,6 +529,9 @@ void mj_saveModel(const mjModel* m, const char* filename, void* buffer, int buff } if (fp) { + if (ferror(fp)) { + mju_warning("Error writing to file '%s'", filename); + } fclose(fp); } } diff --git a/src/ui/ui_main.c b/src/ui/ui_main.c index ffc2fca24e..887d870c3a 100644 --- a/src/ui/ui_main.c +++ b/src/ui/ui_main.c @@ -1894,7 +1894,7 @@ static void shortcuthelp(mjrRect r, int modifier, int shortcut, } // combine - strcat(text, key); + mjSTRNCAT(text, key); // make rectangle for shortcut int g_textver = SCL(ui->spacing.textver, con);