[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[leafnode-list] BeroList 2.5.9 Code Quality Is A Disaster



-----BEGIN PGP SIGNED MESSAGE-----

I wrote to the leafnode mailing list:

> krasel@xxxxxxxxxxxxxxxxxxxxxxxxxxxx (Cornelius Krasel) writes:
> 
> > My guess would be that BeroList eats a multiline Content-Type header.
> 
> The test mail I sent to the list, with the Subject "IGNORE
> THIS<LF><HTAB>test mail for...", supports this, the folded part of the
> header line is visible in an X-Received: line, the Subject has been
> truncated.

I tried to figure if it was BeroList from its sources, I got 2.5.9 and
unpacked it, and looking around, here's what I found:

BEROLIST 2.5.9 IS A MAJOR RELIABILITY DISASTER. I did not check if it's
security relevant, it trashes mail with folded lines (see below), that's
reason enough not to use it.



Elaborate discussion:

While it may or may not be prone to buffer overruns (it usually uses a
calloc clone + strlen before sprintf, though I did not check
thoroughly), it virtually NEVER checks for return codes e. g. from
fopen. 

To give just one example: Look at the unsubscribe function in list.c,
ll. 695ff: It assumes salloc succeeds (does not check). It opens a file,
assuming it is there (no check if list == 0), assumes it has at least
one line (see the i<=members loop condition on the write file.  It
overwrites that file without backup. Should the disk fill in that
moment, the subscriber list is truncated or lost entirely.

I don't see where it locks against multiple running instances to prevent
trashing around, so there's a race condition if more than one "list"
command are active at the same time.

Should malloc fail (memory exhausted), berolist won't notice and crash.

Should a file be temporarily unaccessible (file descriptors exchausted),
berolist will crash.

Look at list.c:831f:

| 	pos=strstr(header,"\n\n")-header;
| 	header[pos]='\0';

It assumes there's no CR. Should there be a CR, it cannot tell header
from body.

Check out the extract function in tool.c. It does not deal with folded
lines. It does not deal with multiple headers of the same type properly,
but just takes the first one (that may be tolerable, I did not check).



The coding quality in general suggests that the author has now known
what he's been doing. It's a maintainability disaster. Look for
NT_SMTP_SERVER in list.c to get the idea, instead of deriving a
preprocessor directive or a const char delimiter[], #ifdef for every
single command.



These should be reasons enough to NOT use BeroList 2.5.9. I'm not
wasting my time on elaborating more reasons for this recommendation.



I've been unable to get any home page, I retrieved BeroList 2.5.9 from a
SUNSITE/METALAB mirror, ftp.gwdg.de. The MD5 of the archive I retrieved
is:

7d3bb01d68a5b96e4bc83585aee80884 *BeroList-2.5.9.tar.gz

The tar.gz has 71019 bytes.

- -- 
cand.-ing. Matthias Andree

-----BEGIN PGP SIGNATURE-----
Version: 2.6.3i
Charset: noconv
Comment: Processed by Mailcrypt 3.5.5, an Emacs/PGP interface

iQCVAwUBOrzuPSdEoB0mv1ypAQGnVwQAi2cs0G0VOagd0bGos51ahZHtRO99Tqrl
GoYC5PN0qtYwWmm465EisW0HDIwJTz7MMlBNUWP/hESNwbmArlGpekCt0sh51eCt
gOHTT4zAw1+nwH+bG/LMg927gVUIZBeVSRjeVYSERxtYyZP2/wtrup8+wWxktSCH
9AMvY0y+YvQ=
=/ZfN
-----END PGP SIGNATURE-----

-- 
leafnode-list@xxxxxxxxxxxxxxxxxxxxxxxxxxxx -- mailing list for leafnode
To unsubscribe, send mail with "unsubscribe" in the subject to the list