Bound checking signed buffer lengths

[amtal]

Some of the bounds checks look a little strange. For example the following checks will pass if b->len + len or pos + outlen overflow (long long), violating the buffer bounds defined by b->alloc or len respectively:

https://github.com/janmojzis/tinyssh/blob/master/tinyssh/buf.c#L65 https://github.com/janmojzis/tinyssh/blob/master/tinyssh/packetparser.c#L51

Since signed overflow is undefined, compiling at -O3 without -fwrapv may optimize away a naive test for a+b<0 since both values are checked to be positive by the previous line. (I haven't gotten that to happen on my machine, though.)

The overall codebase is minimal and simple, so these conditions may be unreachable. Still, why not use size_t instead of long long?

[janmojzis]

Thanks for report! It would have to be very fatal bug in the code, in order to overflow 'long long'. But security first:

I have added compiler parameter -fwrapv -fno-strict-overflow. https://github.com/janmojzis/tinyssh/commit/ff73f24081b8138fc52235576a19952d8271f026 https://github.com/janmojzis/tinyssh/commit/6faff0929fa40620fa7305e49df37a46c50f8736

I have added extra upper bound check in buf.c packetparser.c. Values are checked that they are within the limits <0 - 1073741824>. It makes overflows imposible on systems, where sizeof(long long) >= 4. https://github.com/janmojzis/tinyssh/commit/1802ca3c7978aeee65e0a7f44fb0e1c37a384c56 https://github.com/janmojzis/tinyssh/commit/c633481d6f5f968aad60b97a7ad2ea43e9b30dbd https://github.com/janmojzis/tinyssh/commit/166de29216502b99c4672d8749407ed0967aa17e

Note to size_t: I'm not using size_t in my software, i'm trying to folow rule 'int should be always bigint', and biggest usable integer type is 'long long'. i386: sizeof(size_t)=4, sizeof(long long)=8 amd64: sizeof(size_t)=8, sizeof(long long)=8 arm(RPI2): sizeof(size_t)=4, sizeof(long long)=8

---

Jan