Crash of inn 2.5.1pre r8634 in the keywords code

[Russ Allbery]

Nix <nix at esperi.org.uk> writes:

> So I upgraded my ancient INN 1.7.1 to 2.5.1pre on Sunday... and last
> night it coredumped just after midnight. It seems to have died inside
> the keywords code, which is odd, 'cos I've done a total overview rebuild
> of this entire news server (several million articles) and it never
> crashed then. So this could be a sign of something deeper. (I'm not
> actually using the keywords yet, but expect to use them soon.)

We're not passing a nul-terminated string into KEYgenerate.  This has
probably been broken since some code restructuring for article handling in
the 2.5 development series.  We don't test keyword generation very well.

Try this:

Index: innd/keywords.c
===================================================================
--- innd/keywords.c	(revision 8641)
+++ innd/keywords.c	(working copy)
@@ -26,7 +26,8 @@
 #if !DO_KEYWORDS
 void
 KEYgenerate(HDRCONTENT *header UNUSED, const char *body UNUSED,
-            const char *orig UNUSED, size_t length UNUSED)
+            size_t bodylen UNUSED, const char *orig UNUSED,
+            size_t length UNUSED)
 {
 }
 
@@ -89,13 +90,14 @@
 
 void
 KEYgenerate(
-    HDRCONTENT	*hc,	/* header data */
-    const char	*body,	/* article body */
-    const char	*v,	/* old kw value */
-    size_t	l)	/* old kw length */
+    HDRCONTENT	*hc,		/* header data */
+    const char	*body,		/* article body */
+    size_t      bodylen,	/* article body length */
+    const char	*v,		/* old kw value */
+    size_t	l)		/* old kw length */
 {
 
-    int		word_count, word_length, bodylen, word_index, distinct_words;
+    int		word_count, word_length, word_index, distinct_words;
     int		last;
     char	*text, *orig_text, *text_end, *this_word, *chase, *punc;
     static struct word_entry	*word_vec;
@@ -145,11 +147,14 @@
      * now figure acceptable extents, and copy body to working string.
      * (Memory-intensive for hefty articles: limit to non-ABSURD articles.)
      */
-    bodylen = strlen(body);
     if ((bodylen < 100) || (bodylen > innconf->keyartlimit)) /* too small/big to bother */
 	return;
 
-    orig_text = text = xstrdup(body);	/* orig_text is for free() later on */
+    /* Nul-terminate the body.  orig_text will be freed later. */
+    orig_text = xmalloc(bodylen + 1);
+    memcpy(orig_text, body, bodylen);
+    orig_text[bodylen] = '\0';
+    text = orig_text;
 
     text_end = text + bodylen;
 
Index: innd/art.c
===================================================================
--- innd/art.c	(revision 8641)
+++ innd/art.c	(working copy)
@@ -1886,7 +1886,7 @@
 	key_old_value  = HDR(HDR__KEYWORDS);
 	key_old_length = HDR_LEN(HDR__KEYWORDS);
 	KEYgenerate(&hc[HDR__KEYWORDS], cp->In.data + data->Body,
-                    key_old_value, key_old_length);
+                    cp->Next - data->Body, key_old_value, key_old_length);
       }
     }
 
Index: innd/innd.h
===================================================================
--- innd/innd.h	(revision 8641)
+++ innd/innd.h	(working copy)
@@ -744,7 +744,7 @@
 extern void		CCclose(void);
 extern void		CCsetup(void);
 
-extern void             KEYgenerate(HDRCONTENT *, const char *body,
+extern void             KEYgenerate(HDRCONTENT *, const char *body, size_t,
                                     const char *orig, size_t length);
 
 extern void		LCclose(void);


-- 
Russ Allbery (rra at stanford.edu)             <http://www.eyrie.org/~eagle/>

    Please send questions to the list rather than mailing me directly.
     <http://www.eyrie.org/~eagle/faqs/questions.html> explains why.