[tin-dev] [PATCH] tin: do not call wait_message() without printf format string

[Stefan Assmann]

While downloading articles from gmane.linux.drivers.e1000.devel I ran
into the following segfault.

tin -S -v -c -r -f ~/.newsrc
[...]
[ 1867]  As far as I knew, her closest affiliation with the departed was
that his son Johnny knocked up her daughter Lisa in the confines of
commitment, yielding the beautiful podling Eva.
[ 1869]  MDaemon Notification -- Attachment Removed

tin: signal handler caught SIGSEGV signal (11).
tin 2.2.1 20140504 ("Tober an Righ") [UNIX]: send a DETAILED bug report
to
tin-bugs at tin.org
Aborted (core dumped)

Running the same command in gdb showed a call chain of
check_start_save_any_news()
->wait_message()
  ->fmt_message()

Also note that subject of the article that crashed contained a "%".
Looking through the code revealed that when tin is executed with the -v
option there a several cases where wait_message() is called without
specifying a printf format string and just providing the buf pointer.
Now if buf contains a "%" that is being evaluated as printf format
specifier and BAD things start happening.

This bug is currently present in tin stable (2.2.1) as well as
unstable (2.3.1).

Fix this by providing "%s" as printf format string. Also added an
attribute to the prototype of wait_message() which should warn about
this in the future.

Signed-off-by: Stefan Assmann <sassmann at kpanic.de>
---
 include/proto.h | 3 ++-
 src/save.c      | 6 +++---
 src/select.c    | 2 +-
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/proto.h b/include/proto.h
index 0596af2..f414531 100644
--- a/include/proto.h
+++ b/include/proto.h
@@ -600,7 +600,8 @@ extern void show_progress(const char *txt, t_artnum count, t_artnum total);
 extern void show_title(const char *title);
 extern void spin_cursor(void);
 extern void stow_cursor(void);
-extern void wait_message(unsigned int sdelay, const char *fmt, ...);
+extern void wait_message(unsigned int sdelay, const char *fmt, ...)
+			 __attribute__ ((__format__ (__printf__, 2, 3)));
 
 /* search.c */
 extern enum option_enum search_config(t_bool forward, t_bool repeat, enum option_enum current, enum option_enum last);
diff --git a/src/save.c b/src/save.c
index 9ed557b..decff56 100644
--- a/src/save.c
+++ b/src/save.c
@@ -202,7 +202,7 @@ check_start_save_any_news(
 			snprintf(buf, sizeof(buf), _(txt_saved_groupname), group->name);
 			fprintf(fp_log, "%s", buf);
 			if (verbose)
-				wait_message(0, buf);
+				wait_message(0, "%s", buf);
 
 			if (function == SAVE_ANY_NEWS) {
 				char tmp[PATH_LEN];
@@ -287,7 +287,7 @@ check_start_save_any_news(
 					snprintf(buf, sizeof(buf), "[%5"T_ARTNUM_PFMT"]  %s\n", arts[j].artnum, arts[j].subject);
 					fprintf(fp_log, "%s", buf);	/* buf may contain % */
 					if (verbose)
-						wait_message(0, buf);
+						wait_message(0, "%s", buf);
 
 					while ((line = tin_fgets(artfp, FALSE)) != NULL)
 						fprintf(savefp, "%s\n", line);		/* TODO: error handling */
@@ -344,7 +344,7 @@ check_start_save_any_news(
 					group_count, PLURAL(group_count, txt_group));
 			fprintf(fp_log, "%s", buf);
 			if (verbose)
-				wait_message(0, buf);
+				wait_message(0, "%s", buf);
 
 			if (log_opened) {
 				fclose(fp_log);
diff --git a/src/select.c b/src/select.c
index 82e1a38..0250455 100644
--- a/src/select.c
+++ b/src/select.c
@@ -1307,7 +1307,7 @@ subscribe_pattern(
 		return;
 	}
 
-	wait_message(0, message);
+	wait_message(0, "%s", message);
 
 	for_each_group(i) {
 		if (match_group_list(active[i].name, buf)) {
-- 
2.4.3