Make sure logs are always ended with newline

native/src/base/compat/compat.cpp

@@ -146,6 +146,8 @@ int sigemptyset(sigset_t *set) {
     return 0;
 }
 
+#undef vsnprintf
+#undef snprintf
 #include "fortify.hpp"
 
 #endif

native/src/base/files.cpp

@@ -10,7 +10,7 @@
 using namespace std;
 
 ssize_t fd_path(int fd, char *path, size_t size) {
-    snprintf(path, size, "/proc/self/fd/%d", fd);
+    ssprintf(path, size, "/proc/self/fd/%d", fd);
     return xreadlink(path, path, size);
 }
 

native/src/base/logging.cpp

@@ -13,7 +13,7 @@ using namespace std;
 
 static int fmt_and_log_with_rs(LogLevel level, const char *fmt, va_list ap) {
     char buf[4096];
-    int ret = vsnprintf(buf, sizeof(buf), fmt, ap);
+    int ret = vssprintf(buf, sizeof(buf), fmt, ap);
     log_with_rs(level, rust::Str(buf, ret));
     return ret;
 }
@@ -41,10 +41,15 @@ extern "C" int magisk_log_print(int prio, const char *tag, const char *fmt, ...)
     }
 
     char fmt_buf[4096];
-    auto len = strlcpy(fmt_buf, tag, sizeof(fmt_buf));
+    auto len = strlcpy(fmt_buf, tag, sizeof(fmt_buf) - 1);
     // Prevent format specifications in the tag
     std::replace(fmt_buf, fmt_buf + len, '%', '_');
-    snprintf(fmt_buf + len, sizeof(fmt_buf) - len, ": %s", fmt);
+    len = ssprintf(fmt_buf + len, sizeof(fmt_buf) - len - 1, ": %s", fmt) + len;
+    // Ensure the fmt string always ends with newline
+    if (fmt_buf[len - 1] != '\n') {
+        fmt_buf[len] = '\n';
+        fmt_buf[len + 1] = '\0';
+    }
     va_list argv;
     va_start(argv, fmt);
     int ret = cpp_logger(level, fmt_buf, argv);

native/src/base/misc.cpp

@@ -168,7 +168,7 @@ uint32_t binary_gcd(uint32_t u, uint32_t v) {
 
 int switch_mnt_ns(int pid) {
     char mnt[32];
-    snprintf(mnt, sizeof(mnt), "/proc/%d/ns/mnt", pid);
+    ssprintf(mnt, sizeof(mnt), "/proc/%d/ns/mnt", pid);
     if (access(mnt, R_OK) == -1) return 1; // Maybe process died..
 
     int fd, ret;
@@ -211,3 +211,16 @@ vector<string> split(const string &s, const string &delims) {
 vector<string_view> split_ro(string_view s, string_view delims) {
     return split_impl<string_view>(s, delims);
 }
+
+#undef vsnprintf
+int vssprintf(char *dest, size_t size, const char *fmt, va_list ap) {
+    return std::min(vsnprintf(dest, size, fmt, ap), (int) size - 1);
+}
+
+int ssprintf(char *dest, size_t size, const char *fmt, ...) {
+    va_list va;
+    va_start(va, fmt);
+    int r = vssprintf(dest, size, fmt, va);
+    va_end(va);
+    return r;
+}

native/src/base/misc.hpp

@@ -156,6 +156,15 @@ std::string &replace_all(std::string &str, std::string_view from, std::string_vi
 std::vector<std::string> split(const std::string &s, const std::string &delims);
 std::vector<std::string_view> split_ro(std::string_view, std::string_view delims);
 
+// Similar to vsnprintf, but the return value is the written number of bytes
+int vssprintf(char *dest, size_t size, const char *fmt, va_list ap);
+// Similar to snprintf, but the return value is the written number of bytes
+int ssprintf(char *dest, size_t size, const char *fmt, ...);
+
+// Ban usage of unsafe cstring functions
+#define vsnprintf  __use_vssprintf_instead__
+#define snprintf   __use_ssprintf_instead__
+
 struct exec_t {
     bool err = false;
     int fd = -2;

native/src/core/bootstages.cpp

@@ -26,8 +26,8 @@ bool zygisk_enabled = false;
 
 #define MNT_DIR_IS(dir) (me->mnt_dir == string_view(dir))
 #define MNT_TYPE_IS(type) (me->mnt_type == string_view(type))
-#define SETMIR(b, part) snprintf(b, sizeof(b), "%s/" MIRRDIR "/" #part, MAGISKTMP.data())
+#define SETMIR(b, part) ssprintf(b, sizeof(b), "%s/" MIRRDIR "/" #part, MAGISKTMP.data())
-#define SETBLK(b, part) snprintf(b, sizeof(b), "%s/" BLOCKDIR "/" #part, MAGISKTMP.data())
+#define SETBLK(b, part) ssprintf(b, sizeof(b), "%s/" BLOCKDIR "/" #part, MAGISKTMP.data())
 
 #define do_mount_mirror(part) {     \
     SETMIR(buf1, part);             \

native/src/core/daemon.cpp

@@ -286,13 +286,13 @@ done:
 
 static void switch_cgroup(const char *cgroup, int pid) {
     char buf[32];
-    snprintf(buf, sizeof(buf), "%s/cgroup.procs", cgroup);
+    ssprintf(buf, sizeof(buf), "%s/cgroup.procs", cgroup);
     if (access(buf, F_OK) != 0)
         return;
     int fd = xopen(buf, O_WRONLY | O_APPEND | O_CLOEXEC);
     if (fd == -1)
         return;
-    snprintf(buf, sizeof(buf), "%d\n", pid);
+    ssprintf(buf, sizeof(buf), "%d\n", pid);
     xwrite(fd, buf, strlen(buf));
     close(fd);
 }

native/src/core/db.cpp

@@ -332,7 +332,7 @@ int get_db_settings(db_settings &cfg, int key) {
     };
     if (key >= 0) {
         char query[128];
-        snprintf(query, sizeof(query), "SELECT * FROM settings WHERE key='%s'", DB_SETTING_KEYS[key]);
+        ssprintf(query, sizeof(query), "SELECT * FROM settings WHERE key='%s'", DB_SETTING_KEYS[key]);
         err = db_exec(query, settings_cb);
     } else {
         err = db_exec("SELECT * FROM settings", settings_cb);
@@ -350,7 +350,7 @@ int get_db_strings(db_strings &str, int key) {
     };
     if (key >= 0) {
         char query[128];
-        snprintf(query, sizeof(query), "SELECT * FROM strings WHERE key='%s'", DB_STRING_KEYS[key]);
+        ssprintf(query, sizeof(query), "SELECT * FROM strings WHERE key='%s'", DB_STRING_KEYS[key]);
         err = db_exec(query, string_cb);
     } else {
         err = db_exec("SELECT * FROM strings", string_cb);
@@ -362,7 +362,7 @@ int get_db_strings(db_strings &str, int key) {
 void rm_db_strings(int key) {
     char *err;
     char query[128];
-    snprintf(query, sizeof(query), "DELETE FROM strings WHERE key == '%s'", DB_STRING_KEYS[key]);
+    ssprintf(query, sizeof(query), "DELETE FROM strings WHERE key == '%s'", DB_STRING_KEYS[key]);
     err = db_exec(query);
     db_err_cmd(err, return);
 }

native/src/core/logging.cpp

@@ -114,7 +114,7 @@ static void *logfile_writer(void *arg) {
         }
         long ms = tv.tv_usec / 1000;
         size_t off = strftime(aux, sizeof(aux), "%m-%d %T", &tm);
-        off += snprintf(aux + off, sizeof(aux) - off,
+        off += ssprintf(aux + off, sizeof(aux) - off,
                 ".%03ld %5d %5d %c : ", ms, meta.pid, meta.tid, type);
 
         iov[0].iov_len = off;

native/src/core/package.cpp

@@ -100,7 +100,7 @@ int get_manager(int user_id, string *pkg, bool install) {
 
     auto check_dyn = [&](int u) -> bool {
 #if ENFORCE_SIGNATURE
-        snprintf(app_path, sizeof(app_path),
+        ssprintf(app_path, sizeof(app_path),
             "%s/%d/%s/dyn/current.apk", APP_DATA_DIR, u, mgr_pkg->data());
         int dyn = open(app_path, O_RDONLY | O_CLOEXEC);
         if (dyn < 0) {
@@ -124,7 +124,7 @@ int get_manager(int user_id, string *pkg, bool install) {
         }
         // Just need to check whether the app is installed in the user
         const char *name = mgr_pkg->empty() ? JAVA_PACKAGE_NAME : mgr_pkg->data();
-        snprintf(app_path, sizeof(app_path), "%s/%d/%s", APP_DATA_DIR, user_id, name);
+        ssprintf(app_path, sizeof(app_path), "%s/%d/%s", APP_DATA_DIR, user_id, name);
         if (access(app_path, F_OK) == 0) {
             // Always check dyn signature for repackaged app
             if (!mgr_pkg->empty() && !check_dyn(user_id))
@@ -165,7 +165,7 @@ int get_manager(int user_id, string *pkg, bool install) {
 
             bool invalid = false;
             auto check_pkg = [&](int u) -> bool {
-                snprintf(app_path, sizeof(app_path),
+                ssprintf(app_path, sizeof(app_path),
                          "%s/%d/%s", APP_DATA_DIR, u, str[SU_MANAGER].data());
                 if (stat(app_path, &st) == 0) {
                     int app_id = to_app_id(st.st_uid);
@@ -223,7 +223,7 @@ int get_manager(int user_id, string *pkg, bool install) {
 
         bool invalid = false;
         auto check_pkg = [&](int u) -> bool {
-            snprintf(app_path, sizeof(app_path), "%s/%d/" JAVA_PACKAGE_NAME, APP_DATA_DIR, u);
+            ssprintf(app_path, sizeof(app_path), "%s/%d/" JAVA_PACKAGE_NAME, APP_DATA_DIR, u);
             if (stat(app_path, &st) == 0) {
 #if ENFORCE_SIGNATURE
                 string apk = find_apk_path(JAVA_PACKAGE_NAME);

native/src/core/scripting.cpp

@@ -164,7 +164,7 @@ void install_apk(const char *apk) {
         .fork = fork_no_orphan
     };
     char cmds[sizeof(install_script) + 4096];
-    snprintf(cmds, sizeof(cmds), install_script, apk, JAVA_PACKAGE_NAME);
+    ssprintf(cmds, sizeof(cmds), install_script, apk, JAVA_PACKAGE_NAME);
     exec_command_sync(exec, "/system/bin/sh", "-c", cmds);
 }
 
@@ -179,7 +179,7 @@ void uninstall_pkg(const char *pkg) {
         .fork = fork_no_orphan
     };
     char cmds[sizeof(uninstall_script) + 256];
-    snprintf(cmds, sizeof(cmds), uninstall_script, pkg);
+    ssprintf(cmds, sizeof(cmds), uninstall_script, pkg);
     exec_command_sync(exec, "/system/bin/sh", "-c", cmds);
 }
 
@@ -195,7 +195,7 @@ void clear_pkg(const char *pkg, int user_id) {
         .fork = fork_no_orphan
     };
     char cmds[sizeof(clear_script) + 288];
-    snprintf(cmds, sizeof(cmds), clear_script, pkg, user_id);
+    ssprintf(cmds, sizeof(cmds), clear_script, pkg, user_id);
     exec_command_sync(exec, "/system/bin/sh", "-c", cmds);
 }
 

native/src/init/getinfo.cpp

@@ -125,7 +125,7 @@ extern "C" void klog_write(const char *msg, int len) {
 static int klog_with_rs(LogLevel level, const char *fmt, va_list ap) {
     char buf[4096];
     strlcpy(buf, "magiskinit: ", sizeof(buf));
-    int len = vsnprintf(buf + 12, sizeof(buf) - 12, fmt, ap) + 12;
+    int len = vssprintf(buf + 12, sizeof(buf) - 12, fmt, ap) + 12;
     log_with_rs(level, rust::Str(buf, len));
     return len;
 }
@@ -211,7 +211,7 @@ void BootConfig::print() {
 }
 
 #define read_dt(name, key)                                          \
-snprintf(file_name, sizeof(file_name), "%s/" name, config->dt_dir); \
+ssprintf(file_name, sizeof(file_name), "%s/" name, config->dt_dir); \
 if (access(file_name, R_OK) == 0) {                                 \
     string data = full_read(file_name);                             \
     if (!data.empty()) {                                            \

native/src/init/selinux.cpp

@@ -71,7 +71,7 @@ bool MagiskInit::hijack_sepolicy() {
         // This only happens on Android 8.0 - 9.0
 
         char buf[4096];
-        snprintf(buf, sizeof(buf), "%s/fstab/compatible", config->dt_dir);
+        ssprintf(buf, sizeof(buf), "%s/fstab/compatible", config->dt_dir);
         dt_compat = full_read(buf);
         if (dt_compat.empty()) {
             // Device does not do early mount and uses monolithic policy
@@ -121,7 +121,7 @@ bool MagiskInit::hijack_sepolicy() {
         int fd = xopen(MOCK_COMPAT, O_WRONLY);
 
         char buf[4096];
-        snprintf(buf, sizeof(buf), "%s/fstab/compatible", config->dt_dir);
+        ssprintf(buf, sizeof(buf), "%s/fstab/compatible", config->dt_dir);
         xumount2(buf, MNT_DETACH);
 
         hijack();

native/src/resetprop/persist.cpp

@@ -142,7 +142,7 @@ static void pb_getprop(prop_cb *prop_cb) {
 
 static bool file_getprop(const char *name, char *value) {
     char path[4096];
-    snprintf(path, sizeof(path), PERSISTENT_PROPERTY_DIR "/%s", name);
+    ssprintf(path, sizeof(path), PERSISTENT_PROPERTY_DIR "/%s", name);
     int fd = open(path, O_RDONLY | O_CLOEXEC);
     if (fd < 0)
         return false;
@@ -230,7 +230,7 @@ bool persist_deleteprop(const char *name) {
         return false;
     } else {
         char path[4096];
-        snprintf(path, sizeof(path), PERSISTENT_PROPERTY_DIR "/%s", name);
+        ssprintf(path, sizeof(path), PERSISTENT_PROPERTY_DIR "/%s", name);
         if (unlink(path) == 0) {
             LOGD("resetprop: unlink [%s]\n", path);
             return true;

native/src/su/connect.cpp

@@ -52,7 +52,7 @@ public:
         switch (type) {
         case INT:
             vec.push_back("--ei");
-            snprintf(buf, sizeof(buf), "%d", int_val);
+            ssprintf(buf, sizeof(buf), "%d", int_val);
             str = buf;
             val = str.data();
             break;
@@ -75,7 +75,7 @@ public:
         switch (type) {
         case INT:
             str += ":i:";
-            snprintf(buf, sizeof(buf), "%d", int_val);
+            ssprintf(buf, sizeof(buf), "%d", int_val);
             str += buf;
             break;
         case BOOL:
@@ -114,13 +114,13 @@ static void exec_cmd(const char *action, vector<Extra> &data,
     char exe[128];
     char target[128];
     char user[4];
-    snprintf(user, sizeof(user), "%d", to_user_id(info->eval_uid));
+    ssprintf(user, sizeof(user), "%d", to_user_id(info->eval_uid));
 
     if (zygisk_enabled) {
 #if defined(__LP64__)
-        snprintf(exe, sizeof(exe), "/proc/self/fd/%d", app_process_64);
+        ssprintf(exe, sizeof(exe), "/proc/self/fd/%d", app_process_64);
 #else
-        snprintf(exe, sizeof(exe), "/proc/self/fd/%d", app_process_32);
+        ssprintf(exe, sizeof(exe), "/proc/self/fd/%d", app_process_32);
 #endif
     } else {
         strlcpy(exe, "/system/bin/app_process", sizeof(exe));
@@ -128,7 +128,7 @@ static void exec_cmd(const char *action, vector<Extra> &data,
 
     // First try content provider call method
     if (provider) {
-        snprintf(target, sizeof(target), "content://%s.provider", info->mgr_pkg.data());
+        ssprintf(target, sizeof(target), "content://%s.provider", info->mgr_pkg.data());
         vector<const char *> args{ CALL_PROVIDER };
         for (auto &e : data) {
             e.add_bind(args);
@@ -165,7 +165,7 @@ static void exec_cmd(const char *action, vector<Extra> &data,
 
     // Finally, fallback to start activity with component name
     args[4] = "-n";
-    snprintf(target, sizeof(target), "%s/com.topjohnwu.magisk.ui.surequest.SuRequestActivity", info->mgr_pkg.data());
+    ssprintf(target, sizeof(target), "%s/com.topjohnwu.magisk.ui.surequest.SuRequestActivity", info->mgr_pkg.data());
     exec.fd = -2;
     exec.fork = fork_dont_care;
     exec_command(exec);

native/src/su/su_daemon.cpp

@@ -64,7 +64,7 @@ void su_info::check_db() {
 
     if (eval_uid > 0) {
         char query[256], *err;
-        snprintf(query, sizeof(query),
+        ssprintf(query, sizeof(query),
             "SELECT policy, logging, notification FROM policies "
             "WHERE uid=%d AND (until=0 OR until>%li)", eval_uid, time(nullptr));
         err = db_exec(query, [&](db_row &row) -> bool {
@@ -125,7 +125,7 @@ bool uid_granted_root(int uid) {
     bool granted = false;
 
     char query[256], *err;
-    snprintf(query, sizeof(query),
+    ssprintf(query, sizeof(query),
         "SELECT policy FROM policies WHERE uid=%d AND (until=0 OR until>%li)",
         uid, time(nullptr));
     err = db_exec(query, [&](db_row &row) -> bool {
@@ -158,7 +158,7 @@ void prune_su_access() {
     db_err_cmd(err, return);
 
     for (int uid : rm_uids) {
-        snprintf(query, sizeof(query), "DELETE FROM policies WHERE uid == %d", uid);
+        ssprintf(query, sizeof(query), "DELETE FROM policies WHERE uid == %d", uid);
         // Don't care about errors
         db_exec(query);
     }
@@ -410,11 +410,11 @@ void su_daemon_handler(int client, const sock_cred *cred) {
     // Setup environment
     umask(022);
     char path[32];
-    snprintf(path, sizeof(path), "/proc/%d/cwd", ctx.pid);
+    ssprintf(path, sizeof(path), "/proc/%d/cwd", ctx.pid);
     char cwd[PATH_MAX];
     if (realpath(path, cwd))
         chdir(cwd);
-    snprintf(path, sizeof(path), "/proc/%d/environ", ctx.pid);
+    ssprintf(path, sizeof(path), "/proc/%d/environ", ctx.pid);
     auto env = full_read(path);
     clearenv();
     for (size_t pos = 0; pos < env.size(); ++pos) {

native/src/zygisk/deny/utils.cpp

@@ -71,7 +71,7 @@ static void update_pkg_uid(const string &pkg, bool remove) {
     char buf[PATH_MAX] = {0};
     // For each user
     while ((entry = xreaddir(data_dir.get()))) {
-        snprintf(buf, sizeof(buf), "%s/%s", entry->d_name, pkg.data());
+        ssprintf(buf, sizeof(buf), "%s/%s", entry->d_name, pkg.data());
         if (fstatat(dirfd(data_dir.get()), buf, &st, 0) == 0) {
             int app_id = to_app_id(st.st_uid);
             if (remove) {
@@ -242,7 +242,7 @@ static int add_list(const char *pkg, const char *proc) {
 
     // Add to database
     char sql[4096];
-    snprintf(sql, sizeof(sql),
+    ssprintf(sql, sizeof(sql),
             "INSERT INTO denylist (package_name, process) VALUES('%s', '%s')", pkg, proc);
     char *err = db_exec(sql);
     db_err_cmd(err, return DenyResponse::ERROR)
@@ -286,9 +286,9 @@ static int rm_list(const char *pkg, const char *proc) {
 
     char sql[4096];
     if (proc[0] == '\0')
-        snprintf(sql, sizeof(sql), "DELETE FROM denylist WHERE package_name='%s'", pkg);
+        ssprintf(sql, sizeof(sql), "DELETE FROM denylist WHERE package_name='%s'", pkg);
     else
-        snprintf(sql, sizeof(sql),
+        ssprintf(sql, sizeof(sql),
                 "DELETE FROM denylist WHERE package_name='%s' AND process='%s'", pkg, proc);
     char *err = db_exec(sql);
     db_err_cmd(err, return DenyResponse::ERROR)

native/src/zygisk/entry.cpp

@@ -161,7 +161,7 @@ static vector<int> get_module_fds(bool is_64_bit) {
 }
 
 static bool get_exe(int pid, char *buf, size_t sz) {
-    snprintf(buf, sz, "/proc/%d/exe", pid);
+    ssprintf(buf, sz, "/proc/%d/exe", pid);
     return xreadlink(buf, buf, sz) > 0;
 }
 
@@ -191,7 +191,7 @@ static void connect_companion(int client, bool is_64_bit) {
             // This fd has to survive exec
             fcntl(fds[1], F_SETFD, 0);
             char buf[16];
-            snprintf(buf, sizeof(buf), "%d", fds[1]);
+            ssprintf(buf, sizeof(buf), "%d", fds[1]);
             execl(exe.data(), "", "zygisk", "companion", buf, (char *) nullptr);
             exit(-1);
         }
@@ -340,7 +340,7 @@ static void get_process_info(int client, const sock_cred *cred) {
         if (!as_const(bits)[id]) {
             // Either not a zygisk module, or incompatible
             char buf[4096];
-            snprintf(buf, sizeof(buf), MODULEROOT "/%s/zygisk",
+            ssprintf(buf, sizeof(buf), MODULEROOT "/%s/zygisk",
                 module_list->operator[](id).name.data());
             if (int dirfd = open(buf, O_RDONLY | O_CLOEXEC); dirfd >= 0) {
                 close(xopenat(dirfd, "unloaded", O_CREAT | O_RDONLY, 0644));
@@ -363,7 +363,7 @@ static void send_log_pipe(int fd) {
 static void get_moddir(int client) {
     int id = read_int(client);
     char buf[4096];
-    snprintf(buf, sizeof(buf), MODULEROOT "/%s", module_list->operator[](id).name.data());
+    ssprintf(buf, sizeof(buf), MODULEROOT "/%s", module_list->operator[](id).name.data());
     int dfd = xopen(buf, O_RDONLY | O_CLOEXEC);
     send_fd(client, dfd);
     close(dfd);

native/src/zygisk/main.cpp

@@ -44,7 +44,7 @@ int app_process_main(int argc, char *argv[]) {
         if (fork_dont_care() == 0) {
             // This fd has to survive exec
             fcntl(fds[1], F_SETFD, 0);
-            snprintf(buf, sizeof(buf), "%d", fds[1]);
+            ssprintf(buf, sizeof(buf), "%d", fds[1]);
 #if defined(__LP64__)
             execlp("magisk", "", "zygisk", "passthrough", buf, "1", (char *) nullptr);
 #else