Bugzilla Bug 125990

---------------------------------
A] invalid memory access (32 bit)
---------------------------------

ENet uses 32 bit numbers for almost all the parameters in its packets,
like fragments offset, data size, timestamps, challenge numbers and so
on.
Each packet received by the library (enet_host_service) is handled by
the enet_protocol_handle_incoming_commands function.
This function uses a pointer (currentData) which points to the current
command, each packet can contain one or more commands which describe
operations like a connection request, an acknowledge, a fragment, a
message and more.
The instruction which checks this pointer to avoid that it points over
the received packet can be eluded through a big (negative on 32 bit
CPU) header.commandLength parameter.
After having bypassed the check currentData will point to an invalid
zone of the memory and when the cycle will continue on the subsequent
command (commandCount must be major than one) the application will
crash.
64 bit CPUs should be not vulnerable.

From enet_protocol_handle_incoming_commands in protocol.c:
    ...
    currentData = host -> receivedData + sizeof (ENetProtocolHeader);

    while (commandCount > 0 &&
           currentData < & host -> receivedData [host -> receivedDataLength])
    {
       command = (ENetProtocol *) currentData;

       if (currentData + sizeof (ENetProtocolCommandHeader) > & host ->
receivedData [host -> receivedDataLength])
         return 0;

       command -> header.commandLength = ENET_NET_TO_HOST_32 (command ->
header.commandLength);

       if (currentData + command -> header.commandLength > & host ->
receivedData [host -> receivedDataLength])
         return 0;

       -- commandCount;
       currentData += command -> header.commandLength;
    ...


---------------------------------
B] allocation abort with fragment
---------------------------------

ENet supports also the handling of fragments used to build the messages
bigger than the receiver's MTU.
When a fragment is received the library allocates the total message
size in memory so it can easily rebuild all the subsequent fragments in
this buffer.
If the total data size specified by the attacker cannot be allocated,
the library calls abort() and all the program terminates.

From enet_protocol_handle_send_fragment in protocol.c:
    ...
       startCommand = enet_peer_queue_incoming_command (peer, 
                                                        & hostCommand, 
                                                        enet_packet_create
(NULL, totalLength, ENET_PACKET_FLAG_RELIABLE),
                                                        fragmentCount);

No fix and no reply from upstream according to the advisory. I think we should
at least hardmask the package, removing it completely might be even better.

Unfortunately, no maintainer - I cc'ed the ones in the changelog. Could you do
it?

masked.  I don't see any reason to keep it in the tree.  As far as I can tell,
it's not used by any packages.

that was fast, thank you. package was never stable, so we dont need a masking
GLSA here. Keeping it as enhancement for now.

Any comments from the rest of the security team if we should go for a complete
remove, or is that none of our business?

ping me in a month and I'll whack it.

it was used by some games (cube?)

cube is masked for security reasons. afaik, cube also shipped its own version
of enet.

(In reply to comment #4)
> ping me in a month and I'll whack it.
> 

ping :)

Have anyone tried to contact the developer again? This library is being used
for The Mana World (tmw in portage) and one of the devs (MrLindeijer) told me
that the author (Lee Salzman) is responding in the mailing lists. The project
is not dead.

tmw will start using this as a dependency soon. It has to stay in the tree,
althought I agree that the current version in portage has to be masked or
removed. So I need to now what is the best way to do this: should everything be
removed from tree and this bug closed till the bug is fixed upstream? or should
this bug stay open till a new, fixed, version appears and post new ebuilds when
neccessary?

Apart from that there is another issue:
There are not current stable releases. I think even the ones in portage are
just checkouts from CVS. He provides a tarball with CVS, but it is not
numbered. Should I contact him to fix this or the maintainers could version it
before uploading to the mirrors?

Created an attachment (id=91867) [edit]
enet-security.patch

I'm not sure if this is an appropriate solution to fix the security problem A]
described in the advisory. But IMO enet should drop the packages that have a
negativ header.commandLength and everything should be fine.

Note: This does not fix security issue B]

(From update of attachment 91867 [edit])
forget about that ...

Created an attachment (id=91869) [edit]
enet-security.patch

and try this :/ Sorry for the useless noise

# Michael Sterrett <mr_bones_@gentoo.org> (12 Mar 2006)
# masked due to security issues: bug #125990
net-libs/enet

About time to die?

Fine by me.

There's a new version of enet (much more new than the one in Portage). Maybe it
solves security issues?

http://enet.bespin.org/

Created an attachment (id=104200) [edit]
enet-1.0.ebuild

Here's the ebuild I used some time ago to get The Mana World working.

*** Bug 158464 has been marked as a duplicate of this bug. ***

games-puzzle/enigma has enet bundled too

Handling enigma and tmw on bug #159356

I added enet-1.0 to portage.

closing now since everything is OK. Feel free to reopen if you disagree. No
GLSA since only ~arched. Thanks everybody.