From fefd92a124c5bef3b465ae84b2ae30d3e5f57f37 Mon Sep 17 00:00:00 2001
From: Byrial Jensen <byrial@vip.cybercity.dk>
Date: Thu, 22 Dec 2011 21:01:43 +0100
Subject: [PATCH] Fix buffer possible overflow when writings tags

PGNTagsStatic() could overflow its buffer and is removed. PGNTags() do the work instead of calling it.
It starts by calculation the required buffer size and then allocate the buffer.
PrintPGNTags() now prints directly to the file without having a buffer.
---
 pgntags.c |  120 ++++++++++++++++++++++++++++++-------------------------------
 1 files changed, 59 insertions(+), 61 deletions(-)

diff --git a/pgntags.c b/pgntags.c
index 52cedba..e7476a5 100644
--- a/pgntags.c
+++ b/pgntags.c
@@ -47,9 +47,6 @@
 #include "backend.h"
 #include "parser.h"
 
-static char *PGNTagsStatic P((GameInfo *));
-
-
 
 /* Parse PGN tags; returns 0 for success or error number
  */
@@ -144,69 +141,29 @@ int ParsePGNTag(tag, gameInfo)
 }
 
 
-
-/* Return a static buffer with a game's data.
- */
-static char *PGNTagsStatic(gameInfo)
-    GameInfo *gameInfo;
-{
-    static char buf[8192];
-    char buf1[MSG_SIZ];
-
-    buf[0] = NULLCHAR;
-
-    snprintf(buf1, MSG_SIZ, "[Event \"%s\"]\n",
-	    gameInfo->event ? gameInfo->event : "?");
-    strcat(buf, buf1);
-    snprintf(buf1, MSG_SIZ, "[Site \"%s\"]\n",
-	    gameInfo->site ? gameInfo->site : "?");
-    strcat(buf, buf1);
-    snprintf(buf1, MSG_SIZ, "[Date \"%s\"]\n",
-	    gameInfo->date ? gameInfo->date : "?");
-    strcat(buf, buf1);
-    snprintf(buf1, MSG_SIZ, "[Round \"%s\"]\n",
-	    gameInfo->round ? gameInfo->round : "-");
-    strcat(buf, buf1);
-    snprintf(buf1, MSG_SIZ, "[White \"%s\"]\n",
-	    gameInfo->white ? gameInfo->white : "?");
-    strcat(buf, buf1);
-    snprintf(buf1, MSG_SIZ, "[Black \"%s\"]\n",
-	    gameInfo->black ? gameInfo->black : "?");
-    strcat(buf, buf1);
-    snprintf(buf1, MSG_SIZ, "[Result \"%s\"]\n", PGNResult(gameInfo->result));
-    strcat(buf, buf1);
-
-    if (gameInfo->whiteRating >= 0 ) {
-	snprintf(buf1, MSG_SIZ, "[WhiteElo \"%d\"]\n", gameInfo->whiteRating );
-	strcat(buf, buf1);
-    }
-    if ( gameInfo->blackRating >= 0 ) {
-	snprintf(buf1, MSG_SIZ, "[BlackElo \"%d\"]\n", gameInfo->blackRating );
-	strcat(buf, buf1);
-    }
-    if (gameInfo->timeControl != NULL) {
-	snprintf(buf1, MSG_SIZ, "[TimeControl \"%s\"]\n", gameInfo->timeControl);
-	strcat(buf, buf1);
-    }
-    if (gameInfo->variant != VariantNormal) {
-        snprintf(buf1, MSG_SIZ, "[Variant \"%s\"]\n", VariantName(gameInfo->variant));
-	strcat(buf, buf1);
-    }
-    if (gameInfo->extraTags != NULL) {
-	strcat(buf, gameInfo->extraTags);
-    }
-    return buf;
-}
-
-
-
 /* Print game info
  */
 void PrintPGNTags(fp, gameInfo)
      FILE *fp;
      GameInfo *gameInfo;
 {
-    fprintf(fp, "%s", PGNTagsStatic(gameInfo));
+    fprintf(fp, "[Event \"%s\"]\n", gameInfo->event ? gameInfo->event : "?");
+    fprintf(fp, "[Site \"%s\"]\n", gameInfo->site ? gameInfo->site : "?");
+    fprintf(fp, "[Date \"%s\"]\n", gameInfo->date ? gameInfo->date : "?");
+    fprintf(fp, "[Round \"%s\"]\n", gameInfo->round ? gameInfo->round : "-");
+    fprintf(fp, "[White \"%s\"]\n", gameInfo->white ? gameInfo->white : "?");
+    fprintf(fp, "[Black \"%s\"]\n", gameInfo->black ? gameInfo->black : "?");
+    fprintf(fp, "[Result \"%s\"]\n", PGNResult(gameInfo->result));
+    if (gameInfo->whiteRating >= 0)
+	fprintf(fp, "[WhiteElo \"%d\"]\n", gameInfo->whiteRating);
+    if (gameInfo->blackRating >= 0)
+	fprintf(fp, "[BlackElo \"%d\"]\n", gameInfo->blackRating);
+    if (gameInfo->timeControl)
+	fprintf(fp, "[TimeControl \"%s\"]\n", gameInfo->timeControl);
+    if (gameInfo->variant != VariantNormal)
+        fprintf(fp, "[Variant \"%s\"]\n", VariantName(gameInfo->variant));
+    if (gameInfo->extraTags)
+	fputs(gameInfo->extraTags, fp);
 }
 
 
@@ -215,7 +172,48 @@ void PrintPGNTags(fp, gameInfo)
 char *PGNTags(gameInfo)
     GameInfo *gameInfo;
 {
-    return StrSave(PGNTagsStatic(gameInfo));
+    size_t len;
+    char *buf;
+    char *p;
+
+    // First calculate the needed buffer size.
+    // Then we don't have to check the buffer size later.
+    len = 12 + 11 + 11 + 12 + 12 + 12 + 25 + 1; // The first 7 tags
+    if (gameInfo->event) len += strlen(gameInfo->event);
+    if (gameInfo->site)  len += strlen(gameInfo->site);
+    if (gameInfo->date)  len += strlen(gameInfo->date);
+    if (gameInfo->round) len += strlen(gameInfo->round);
+    if (gameInfo->white) len += strlen(gameInfo->white);
+    if (gameInfo->black) len += strlen(gameInfo->black);
+    if (gameInfo->whiteRating >= 0) len += 40;
+    if (gameInfo->blackRating >= 0) len += 40;
+    if (gameInfo->timeControl) len += strlen(gameInfo->timeControl) + 20;
+    if (gameInfo->variant != VariantNormal) len += 50;
+    if (gameInfo->extraTags) len += strlen(gameInfo->extraTags);
+
+    buf = malloc(len);
+    if (!buf)
+	return 0;
+
+    p = buf;
+    p += sprintf(p, "[Event \"%s\"]\n", gameInfo->event ? gameInfo->event : "?");
+    p += sprintf(p, "[Site \"%s\"]\n", gameInfo->site ? gameInfo->site : "?");
+    p += sprintf(p, "[Date \"%s\"]\n", gameInfo->date ? gameInfo->date : "?");
+    p += sprintf(p, "[Round \"%s\"]\n", gameInfo->round ? gameInfo->round : "-");
+    p += sprintf(p, "[White \"%s\"]\n", gameInfo->white ? gameInfo->white : "?");
+    p += sprintf(p, "[Black \"%s\"]\n", gameInfo->black ? gameInfo->black : "?");
+    p += sprintf(p, "[Result \"%s\"]\n", PGNResult(gameInfo->result));
+    if (gameInfo->whiteRating >= 0)
+	p += sprintf(p, "[WhiteElo \"%d\"]\n", gameInfo->whiteRating);
+    if (gameInfo->blackRating >= 0)
+	p += sprintf(p, "[BlackElo \"%d\"]\n", gameInfo->blackRating);
+    if (gameInfo->timeControl)
+	p += sprintf(p, "[TimeControl \"%s\"]\n", gameInfo->timeControl);
+    if (gameInfo->variant != VariantNormal)
+        p += sprintf(p, "[Variant \"%s\"]\n", VariantName(gameInfo->variant));
+    if (gameInfo->extraTags)
+	strcpy(p, gameInfo->extraTags);
+    return buf;
 }
 
 
-- 
1.7.0.4