From ff39ce3874c89586913080816bafe8edb7592ea5 Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Wed, 22 May 2024 19:16:16 -0400
Subject: [PATCH] Don't use `sprintf` if there is only one argument

This avoids problems when logging a string that might have
metacharacters.
---
 ddclient.in | 19 ++++++++++---------
 t/logmsg.pl | 18 ++++++++++++++++++
 2 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/ddclient.in b/ddclient.in
index 3ae7cdb..d9d600d 100755
--- a/ddclient.in
+++ b/ddclient.in
@@ -2420,15 +2420,16 @@ sub logmsg {
         }
     }
 }
-sub msg     { logmsg(                               sprintf(shift, @_)); }
-sub verbose { logmsg(email => 1, pfx => pop,        sprintf(shift, @_)) if opt('verbose'); }
-sub info    { logmsg(email => 1, pfx => 'INFO:',    sprintf(shift, @_)) if opt('verbose'); }
-sub debug   { logmsg(            pfx => 'DEBUG:',   sprintf(shift, @_)) if opt('debug'); }
-sub debug2  { logmsg(            pfx => 'DEBUG:',   sprintf(shift, @_)) if opt('debug') && opt('verbose'); }
-sub warning { logmsg(email => 1, pfx => 'WARNING:', sprintf(shift, @_)); }
-sub fatal   { logmsg(email => 1, pfx => 'FATAL:',   sprintf(shift, @_)); sendmail(); exit(1); }
-sub success { logmsg(email => 1, pfx => 'SUCCESS:', sprintf(shift, @_)); }
-sub failed  { logmsg(email => 1, pfx => 'FAILED:',  sprintf(shift, @_)); $result = 'FAILED'; }
+sub _logmsg_fmt { return (@_ > 1) ? sprintf(shift, @_) : shift; }
+sub msg     { logmsg(                               _logmsg_fmt(@_)); }
+sub verbose { logmsg(email => 1, pfx => shift,      _logmsg_fmt(@_)) if opt('verbose'); }
+sub info    { logmsg(email => 1, pfx => 'INFO:',    _logmsg_fmt(@_)) if opt('verbose'); }
+sub debug   { logmsg(            pfx => 'DEBUG:',   _logmsg_fmt(@_)) if opt('debug'); }
+sub debug2  { logmsg(            pfx => 'DEBUG:',   _logmsg_fmt(@_)) if opt('debug') && opt('verbose'); }
+sub warning { logmsg(email => 1, pfx => 'WARNING:', _logmsg_fmt(@_)); }
+sub fatal   { logmsg(email => 1, pfx => 'FATAL:',   _logmsg_fmt(@_)); sendmail(); exit(1); }
+sub success { logmsg(email => 1, pfx => 'SUCCESS:', _logmsg_fmt(@_)); }
+sub failed  { logmsg(email => 1, pfx => 'FAILED:',  _logmsg_fmt(@_)); $result = 'FAILED'; }
 sub prettytime { return scalar(localtime(shift)); }
 
 sub prettyinterval {
diff --git a/t/logmsg.pl b/t/logmsg.pl
index ccc43d2..47bc0cb 100644
--- a/t/logmsg.pl
+++ b/t/logmsg.pl
@@ -118,4 +118,22 @@ for my $tc (@test_cases) {
     }
 }
 
+{
+    my $output;
+    open(my $fh, '>', \$output);
+    local *STDERR = $fh;
+    ddclient::msg('%%');
+    close($fh);
+    is($output, "%%\n", 'single argument is printed directly, not via sprintf');
+}
+
+{
+    my $output;
+    open(my $fh, '>', \$output);
+    local *STDERR = $fh;
+    ddclient::msg('%s', 'foo');
+    close($fh);
+    is($output, "foo\n", 'multiple arguments are formatted via sprintf');
+}
+
 done_testing();