Ian Lance Taylor wrote:
> Consider code along these lines:
>
> struct s { int len; char* p; };
>
> inline char
> bar (struct s *sp, int n)
> {
> if (n < 0)
> abort ();
> if (n > sp->len)
> abort ();
> return sp->p[n];
> }
>
> void
> foo (struct s *sp, int n)
> {
> int len = sp->len;
> int i;
> int tot = 0;
> for (i = 0; i <= len; ++i)
> tot += bar (sp, i);
> return tot;
> }
>
> Let's assume that bar() is inlined into foo(). Now consider the
> assert. If signed overflow is undefined, then we can optimize away
> the "n < 0" test; it will never be true. If signed overflow is
> defined, then we can not optimize that away. That is because as far
> as the compiler knows, sp->len might be INT_MAX. In that case, the
> loop will never terminate, and i will wrap and become negative. (The
> compiler may also eliminate the "n > sp->len" test, but that does not
> rely on undefined signed overflow.)
This is an excellent example of the kind of subtle vulnerabilities
undefined overflow behaviour causes. Consider the case where sp->len is
under the control of an attacker. Let's assume further that sp->p is
dynamically allocated, and we're running in an OS configuration where
malloc(INT_MAX) actually works.
Now an attacker could provoke a situation where sp->len is INT_MAX, and
i becomes negative. All of a sudden, if the "n < 0" test is folded
away, he's left in a situation where memory is accessed that he's not
supposed to. If instead of summing up the array elements the code would
write them to a network socket, we'd be getting a free dump of all the
heap objects in memory after sp. That might be your private key or your
password, if you're unlucky.
Even worse would be the case where bar would write to sp->p. Attackers
writing to memory are always bad news, and if the circumstances are
right, this situation could be exploitable. This is even applicable for
infinite loops: changing the SEH on Windows to point to exploit code
before the infinite loop finally triggers an exception is a popular
exploitation technique on that platform.
See how easy it is to make this kind of security mistake, even for
people who are aware of the undefinedness of signed overflow? Did you
notice how innocently "nothing can happen here" function bar looks?
> This is a typical example of removing an if branch because signed
> overflow is undefined. This kind of code is common enough.
I could not have made my point any better myself.
Andreas
