-2

I'm building mathematics functions that I plan to use for cryptography.

Your algorithm is useless if the code is vulnerable to exploitation. Buffers are fairly easy to protect against overflows; but what about integers?

I won't share my Galois functions but here is one of my normal addition functions:

/**/
private:
    bool ERROR;
    signed int ret;

    void error(std::string msg){
            ERROR = true;
            std::cout<<"\n[-] "<<msg<<std::endl;
    }
/**/
public:
signed int add(signed int a, signed int b){
                if(sizeof(a) > sizeof(signed int) || sizeof(b) > sizeof(signed int) || sizeof(a+b) > sizeof(signed int)){
                        error("Integer overflow!");
                        ERROR = true;
                        ret = 0;
                        return ret;
                }else{
                        ERROR = false;
                        ret = a + b;
                        return ret;
                }
                error("context failure");
                ret = 0;
                ERROR = true;
                return ret;
        }

Is the if conditional enough to prevent malicious input? If not, how would I fix this vulnerability?

Ahab Devoid
  • 29
  • 6
  • 6
    `sizeof` is evaluated at compile-time. It doesn't care anything about the values in the variables, only their types. (So, no, this offers zero protection.) – Mat Aug 24 '18 at 11:59
  • Unlike buffers integers won't overflow in other memory-areas, they just wrp around producing wrong results - and `sizeof(x)` will be the same for any `double x` regardless of `x`'s value. – piet.t Aug 24 '18 at 12:00
  • 1
    if you want to check whether int (a+b) overflows, you should check ( MAX_INT - b) – Andrew Kashpur Aug 24 '18 at 12:00
  • set a predetermined size for the integer as if it's a buffer. – Ahab Devoid Aug 24 '18 at 12:04
  • `std::size` exists. Btw; why don't you use an existing bignum library (like [GMP](https://gmplib.org)) if you need to work with numbers of arbitrary size? – Jesper Juhl Aug 24 '18 at 12:11
  • 1
    @piet.t "integers won't overflow in other memory-areas, they just wrp around producing wrong results" - Not *quite*. *Unsigned* integer overflow is defined to wrap. But *signed* integer overflow is Undefined Behaviour. And per the standard, if your program contains UB *anywhere* your program is invalid and has no meaning and the compiler is allowed to do whatever it wants to your code (and *not* just the bit that contains the UB, but *any* part of the program). So, integer overflow is *not* harmless or even a localised bug. It means you can't trust *anything*. And yes, compilers exploit it. – Jesper Juhl Aug 24 '18 at 12:18
  • @JesperJuhl Wow, didn't know that this was UB - gotta beware of nasal demons.... – piet.t Aug 24 '18 at 12:20
  • I don't think I've ever said this before, but this looks like an *X, Y* problem. I usually leave requirements to the poster and assume they know what they want even if it is a little unusual. But in this case it seems like the wrong questions are being asked. – jww Aug 24 '18 at 12:34
  • @AndrewKashpur What if `b` is `MIN_INT`? `MAX_INT - MIN_INT` is usually not expressable as `int` (without overflow). ;-) – Scheff's Cat Aug 24 '18 at 12:38
  • Because of the way `sizeof` expressions work, the `if()` condition will always evaluate to `false`. This means the `else` block is always executed. If a compiler is smart enough in optimisation, it might eliminate the test entirely, and simply execute the `else` block directly. – Peter Aug 24 '18 at 12:39
  • If you want a low-level integer overflow libraries then checkout safe_iop. Android uses safe_iop for safer C integer operations. If you are using C++, you can use David LeBlanc's SafeInt class. Microsoft uses it for safer integer operations. – jww Aug 24 '18 at 12:39
  • And, BTW, the way to avoid signed integer overflows is to test the values being operated on BEFORE operating on them. For example, do tests involving `a` and `b` BEFORE attempting to add them. Once the overflow has occurred for signed types, the behaviour is undefined. For unsigned types, it is possible to detect overflow by comparing the result with the values it is derived from (for example if `a` and `b` are both unsigned, then `a + b` will be less than one of `a` or `b` if overflow occurs). – Peter Aug 24 '18 at 12:46
  • Could you provide an example of "malicious" input that you have found this to be effective against? – molbdnilo Aug 24 '18 at 13:02
  • @Scheff you are absolutely right. What i wrote is correct for unsigned int, but may not be applied to signed int as is. It is pretty easy to work out similar conditions for signed int, but it should be done as you pointed out. – Andrew Kashpur Aug 24 '18 at 14:13
  • @AndrewKashpur I wrote this with a ";-)" as I would've liked to suggest a working alternative. But I couldn't find a simple alternative (even after thinking a while). IMHO it's not that trivial to detect overflow of `int` addition beforehand. – Scheff's Cat Aug 24 '18 at 14:19
  • @AndrewKashpur ...except to do it in the next bigger type and check the result if it fits into the lower as well. (E.g. to add two `int32_t`s as `int64_t`s and casting the result back to `int32_t` when check passed). Btw. this still leaves the weakness that it's not applicable to the biggest available `int` type... – Scheff's Cat Aug 24 '18 at 14:28
  • 1
    @Scheff if sign(a)!=sign(b) then a+b will not cause overflow. if sign(a)==sign(b)==sign(1) then (MAX_INT - a < b) <=> a+b overflow. if sign(a)==sign(b)==sign(-1) then (MIN_INT - a > b) <=> a+b overflow – Andrew Kashpur Aug 24 '18 at 14:33
  • @AndrewKashpur This looks much more reasonable... (without ;-) - A voice wispered in my back-head that the sign could've some value somehow but I couldn't get it clear.) – Scheff's Cat Aug 24 '18 at 14:37
  • @AndrewKashpur With a bit sample code (with some border cases), this could make a nice answer (considering that the issue of integer overflows was mentioned in the title). – Scheff's Cat Aug 24 '18 at 14:42
  • My head keeps getting stuck when I think about the data that the integer becomes. I'm certain that upon an integer overflowing past the number 4.294.967.295, you get a negative value upon the memory loop around. I have two ideas to solve the problem. limit the int to a 9 digit polynomial or to trash any values that are above 128 and below -128. – Ahab Devoid Aug 24 '18 at 21:57

2 Answers2

2

As per the other answer, no, sizeof won't protect against what you're trying to do. It cares about the byte width of types, not anything else.

You're asking about integer overflow, but your example has doubles. Doubles are floating point, and AFAIK have a well-defined "overflow" cause. In the case of a value that exceeds the maximum value, you'll get +INF, positive infinity. But you'll lose a lot of precision well before that point. Floating point values won't wrap around.

AFAIK, within the current relevant C/C++ standards, there's no way to portably detect an unsigned integer "overflow" (which is well defined), but gcc and clang have builtins to detect one. You can try to predict the unsigned overflow, but the best and most portable methods are still hotly debated.

Signed integer overflow is undefined behavior, meaning the implementation is free to do anything it wants when encountering it.

If you're dead set on rolling your own cryptography against best practices, you should carefully examine what other implementations have done and make sure you understand why.

It's also worth noting that, security-wise, integer/float overflows are NOT the same as buffer overflows.

TheGoddessInari
  • 21
  • 2
  • The C++ standard does not define what happens on floating point overflow - so that is undefined. Some representations (notably IEEE formats, which are pretty common these days) go further than that, and support infinities, but that is not true of all floating point representations. – Peter Aug 24 '18 at 12:35
  • @TheGoddessInari _there's no way to portably detect an unsigned integer "overflow" (which is well defined)_ For `sum = a + b;`, wouldn't `if (sum < a)` detect this? (assuming that `sum`, `a`, and `b` have all the same `unsigned` type) – Scheff's Cat Aug 24 '18 at 15:00
  • I pasted the block of code that uses the doubles because the function that I'm referring to has value that can't just be shared freely(It's also still being worked on and is in a confusing state). I'll adjust the question accordingly. The important part to me is the if() logic that's supposed to sanitize the integer size – Ahab Devoid Aug 24 '18 at 22:01
0

So I've found my answer.

I've decided to use the following if logic to prevent an integer overflow:

if((a >= 2000000000 || a <= -2000000000) ||
  (b >= 2000000000 || b <= -2000000000) || 
  ((a+b) >= 2000000000 || (a+b) <= -2000000000)){

After running some tests i was able to confirm that the integer loops back around into the negative numbers.

Since I'm working with finite fields, I can expect that normal input from the program won't get near 2 million while also ensuring that overflows are handled.

If outside of the bounds, exit.

~edit: Fixed logic error

Ahab Devoid
  • 29
  • 6