[Mldonkey-users] [patch #5371] EDK: Support for eMule comments (read-only)

[z]

Follow-up Comment #12, patch #5371 (project mldonkey):

The commonFile structure is common to all files, hence its
name.  It should be kept as small as possible since it
applies to all files from all networks and sits in memory
for each and every one of them.  (I'm sure some junk is left
over from when mldonkey was only a donkey client though.)

When a network has a feature that is specific to its type of
files it is added to the structure in the specific plugin's
file structure.  It would be a waste to add it to commonFile
(as well as awkward to work with). I understand that it can
work (lots of bad code can "work"),  but it doesn't really
follow the layout of all the existing code.  In this case it
should be added to donkeyTypes.file.  

Example: Network1 may have file comments that include a
string and an int8.  Network2 might have file comments that
are a string and a list of tags along with an int64.
Whatever the structure, it is specific to that one network,
and the file structures of the network should precisely
match the protocol of the network. That's what those
structures are for! :-)

All commonFiles have file_ops, and these file_ops would
allow something like file_ops.op_num_comments to return an
int from all files for your use in the "html vd list -> # of
comments feature", also guiTypes could be used to create
structures that could encompass file comments from all
networks.  These structures are only filled in on demand,
and hence don't waste memory unlike trying to add these
structures to commonFile. (see op_file_info.)

(BTW, file descriptions are only sent when a download is
initiated, so I don't really understand the point of seeing
this stuff in search results)

After scanning the patch, here are is another comment:

Even though ocaml is not prone to normal C-style buffer
overflows, standard defensive coding must be practiced,
especially when receiving data from a network.

What happens if a nefarious client decides to send many
random strings as file descriptions after you request a file
from them.  The code in this patch blindly accepts these
strings and adds them to the list. 

However, since the dummy_file is used as the default
"last_asked_file" and no checks are really performed to
ensure that a valid file request has been made, a file
doesn't even need to be requested before any connected
client could send an infinite number of random string file
descriptions and fill up this file_comment list structure 
in the dummy_file (and therefore all RAM?).  It would seem 
that any mldonkey running with this patch is prone to a 
potential DOS attack.


    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/patch/?5371>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/