Re: [courier-users] A small bug that causes SqWebmail crashed

I've taken quite a few hours to find the origin of this kind of
SqWebmail's crash. Please see the patch at the end of this mail. In fact
only one line's modification is in this patch.

The problem exists in parsetagbuf() of "html.c". The first calling
parseattr() is to obtain how much space is required to contain parsed
tag attribute information. And the second calling parseattr() is
actually to put parsed information into the space determined by first
calling's result. However, the variable "tagattrlen" is ONLY decided by
the first calling parseattr(), and "tagattrlen" will affect following
operations.
Started from revision 1.21 of html.c, three operations that change
tag's buffer("tagbuf") are added into parseattr() and parsetagbuf():
1.20->1.21 memset(tagbuf, ' ', strlen(tagbuf));
1.21->1.22 *q=0;
1.22->1.23 memset(tagbuf, ' ', strlen(tagbuf));
These operations mean that twice's calling parseattr() will probably
obtain different return values.
Now I set an example: In my case, the first calling parseattr()
will make tagattrlen=2. And an UNINITIALIZED 2-item array of structure
tagattrinfo will be allocated. But after allocating the array, "tagbuf"
will be blanked by "memset(...)" for invalid syntax. Then naturally the
second calling parseattr() will find NO tag attributes, but at the same
time the array pointed by "tagattr" will be KEPT UNINITIALIZED YET. If
following operations TOUCH the UNINITIALIZED array pointed by "tagattr"
according to EXPIRED "tagattrlen" (now is 2), SqWebmail will crash.

I don't think that modification on "tagbuf" is of good style. If
invalid syntax is found, procession on current tag should be stopped
immediately. For example, parseattr() returns 0, and then parsetagbuf()
returns error after first calling parseattr(), finally filtered_tag()
returns immediately.

BTW, I'd like to set up a Chinese website on Courier in China just
like www.courier-mta.jp. Will it attain your spiritual support? I am
always using Courier on my site and my employer's (a university) sites.

Current mainstream in China is atheism, and January 1st is only a
minor festival. I have to work these days.


--- /root/courier-0.52.1/webmail/html.c Tue Sep 6 01:08:23 2005
+++ html.c Mon Dec 26 01:15:28 2005
@@ -259,21 +259,21 @@
{
struct tagattrinfo *newta= tagattr ? (struct tagattrinfo *)
realloc(tagattr, (tagattrlen+16)*sizeof(*tagattr))
:(struct tagattrinfo *)
malloc((tagattrlen+16)*sizeof(*tagattr));

if (!newta) enomem();
tagattrsize=tagattrlen+16;
tagattr=newta;
}
- parseattr(tagattr);
+ tagattrlen=parseattr(tagattr);
}

/* See if this attribute is the one we're looking for */

static int is_attr(struct tagattrinfo *i, const char *l)
{
size_t ll=strlen(l);

return (i->tagnamelen == ll && strncasecmp(i->tagname, l, ll) == 0);
}


Sam Varshavchik wrote:

> mag@intron.ac writes:
>
>> When invalid "&..." is removed, the length of NULL-terminated string
>> in "tagbuf" should not be changed.
>
> Why not?
>





-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems? Stop! Download the new AJAX search engine that makes
searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click
_______________________________________________
courier-users mailing list
courier-users@lists.sourceforge.net
Unsubscribe: https://lists.sourceforge.net/lists/.../courier-users