possible overflow in call to messageLengthIsCorrect()

[cfpp2p]

I think there could be a possible overflow in peer-msgs.c when messageLengthIsCorrect() is called.

line 1495

if (!messageLengthIsCorrect (msgs, id, msglen + 1))

When msglen is equal to SIZE_MAX then we would overflow.

Also a possible overflow if pieceCount equals SIZE_MAX line 1387

return len == (msg->torrent->info.pieceCount + 7u) / 8u + 1u;

I'm not sure how or if these could ever happen but I thought it might be worth looking into.

[cfpp2p]

edit:

and anytime the sum of pieceCount + 7 would exceed SIZE_MAX

[cfpp2p]

patch to fix

[mike.dld]

First is not really true. When initialized, msglen is greater or equal to one (message ID size) and less or equal to UINT_MAX (variable type is uint32_t), then decrement is made with an appropriate comment:

--msglen; /* id length */

This means that msglen is now greater or equal to zero and less than UINT_MAX, so incrementing now will not lead to overflow. Adding an assertion on initial "greater than zero" condition may still be a nice change though.

Second part could be true, although not sure how as well.

[mike.dld]

Committed as r14568. Thanks!

[cfpp2p]

Replying to mike.dld:

This means that msglen is now greater or equal to zero and less than UINT_MAX, so incrementing now will not lead to overflow. Adding an assertion on initial "greater than zero" condition may still be a nice change though.

I understand variable type is uint32_t
Considered in function messageLengthIsCorrect would pass true for a value of
INT_MAX at case BT_LTEP:

line 1675 peer-msgs.c
type conversion of msglen ( uint32_t ) downcast to int for call to parseLtep

static void
parseLtep (tr_peerMsgs * msgs, int msglen, struct evbuffer * inbuf)

That's what I was thinking about with the INT_MAX

Second part could be true, although not sure how as well.

I don't see that we need to get carried away with either part :)

[mike.dld]

I don't see the reason for it to be int. Changed to uint32_t in r14569.

[cfpp2p]

Replying to mike.dld:

I don't see the reason for it to be int. Changed to uint32_t in r14569.

r14569 looks good