use of strlcpy on overlapping source and destination

[Florian Schlichting]

Hi Julien,

a little while back we had an issue where a user was denied posting with
"address not in Internet syntax" while using a From address of the form
a at test1.de. Debugging revealed that nnrpd copies a buffer into itself to
look at the part behind the "@", and for very specific inputs on an old
version of nnrpd that still uses strcpy (and our particular libc), the
result did not contain the dot separating the top-level domain any more.

I'm unable to provide a working test case on current versions of nnrpd,
but the From address check still copies overlapping parts of a buffer
using strlcpy (and in INNs replacement implementation, memcpy), which
can lead to undefined results. Fortunately the fix is easy, as making a
copy is actually unnecessary (frombuf is not used later on):


--- a/nnrpd/post.c
+++ b/nnrpd/post.c
@@ -1090,8 +1090,7 @@ ARTpost(char *article, char *idbuff, bool ihave, bool *permanent)
     HeaderCleanFrom(frombuf);
     p = strchr(frombuf, '@');
     if (p) {
-       strlcpy(frombuf, p+1, sizeof(frombuf));
-       p = strrchr(frombuf, '.');
+       p = strrchr(p+1, '.');
        if (!p) {
            if (modgroup)
                free(modgroup);


I couldn't find any similar uses of strlcpy on overlapping source and
destination in nnrpd/post.c, but haven't looked further.

While testing, it occurred to me that the From address check could
easily be improved to check for the existence of at least one character
before the '@' (more checks are certainly possible, but better left to
the posting filter...):


--- a/nnrpd/post.c
+++ b/nnrpd/post.c
@@ -1088,7 +1088,7 @@ ARTpost(char *article, char *idbuff, bool ihave, bool *permanent)
        else
            *p++ = ' ';
     HeaderCleanFrom(frombuf);
-    p = strchr(frombuf, '@');
+    p = strchr(frombuf+1, '@');
     if (p) {
        p = strrchr(p+1, '.');
        if (!p) {


Florian