Bug #52550 [Ana]: integer undefined behaviors executed during "make test"

Edit report at http://bugs.php.net/bug.php?id=52550&edit=1 ID: 52550 Updated by: rasmus@php.net Reported by: regehr at cs dot utah dot edu Summary: integer undefined behaviors executed during "make test" Status: Analyzed Type: Bug Package: *General Issues Operating System: linux PHP Version: trunk-SVN-2010-08-06 (snap) Block user comment: N New Comment: Do you have a way to generate the list with the test case filename that triggered the problem? Previous Comments: ------------------------------------------------------------------------ [2010-08-06 16:57:55] regehr at cs dot utah dot edu To reiterate: this isn't static analysis. Our tool runs your code under your test suite and looks for integer operations that the C standard tells us are undefined. I manually verified a few more of these reported bugs and they were also real. Anyway, I leave the rest to you. Thanks. ------------------------------------------------------------------------ [2010-08-06 16:52:44] regehr at cs dot utah dot edu Hi Rasmus-- You're right, this overflow is not totally obvious. It occurs when '2147483647;' is passed to parse_iv2() in this test: ext/standard/tests/serialize/serialization_miscTypes_001.phpt The first problem is that the signed add in (result * 10 + cursor) overflows; the subtraction overflow is secondary. If you re-associate the expression and instead compute (result * 10 + (cursor - '0')) the problems go away. ------------------------------------------------------------------------ [2010-08-06 08:33:38] rasmus@php.net No, it isn't obvious since that line of code is only executed when the value of cursor is between '0' and '9'. It would require the argument to parse_iv2() to be above MAXINT which re2c checks for earlier. ------------------------------------------------------------------------ [2010-08-06 08:10:22] regehr at cs dot utah dot edu Hi-- It's not static analysis and there are no false positives. These behaviors really happen (unless our tool is buggy, of course this is always possible). Yes, I should have edited out the undefined behavior that occurred during conftest. My copy of var_unserializer.c has this code at line 228: result = result * 10 + cursor - '0'; It looks to me like "result * 10 + cursor" evaluates to -2147483601 and then the subtraction overflow occurs. This is not obvious? Anyway, this is easy to verify: put an appropriate assertion in the code and run "make test" yourself. ------------------------------------------------------------------------ [2010-08-06 08:00:32] rasmus@php.net They don't make much sense to me. Starting from the bottom, conftest.cpp is a test file generated by the autoconf/libtool chain and only used during the configure run. So even if there was an issue in that, it really wouldn't matter since no user data ever gets to it, and even if it did, you are reporting it to the wrong project. Next one up, ext/standard/var_unserializer.c line 228? There is no substraction there. But assuming it means close to line 228, it would be in the parse_iv2() function? I see nothing wrong with that code. The only code in it that does any sort of addition is: if (cursor >= '0' && cursor <= '9') { result = result * 10 + cursor - '0'; How exactly do you get a signed subtraction overflow from that? cursor is constrained and result is initialized to 0. The 3rd last one seems to point at the same code. There may very well be bugs hiding in there, but a static analysis full of false positives isn't very useful. ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at http://bugs.php.net/bug.php?id=52550 -- Edit this bug report at http://bugs.php.net/bug.php?id=52550&edit=1