Re: [PHP-DEV] [PATCH] potential null dereference in ext/ftp/ftp.c
Hello, clang is indeed a great tool but since it does a lot more than just static analysis. For those cases where one wants source code analysis, especially security oriented, I'd recommend flawfinder [http://www.dwheeler.com/flawfinder]. This is a very good tool and it exists in the official repos for Debian, Ubuntu and FC [and probably many others but these I checked]. It can operate on both C and C++ source files [less relevant for the PHP engine but good to know, right?]. I ran it against the PHP 5.2.11 sources and am now sorting through results, patching suggestions may follow :) May the source be with you, Best regards, Jess Portnoy Michael Maclean wrote: Hi, Gwynne pointed me at the clang static analyser earlier on today, and so I've run it against current PHP_5_3. In the course of messing with it, it noticed a potential null dereference in ext/ftp - I've attached a one-liner to fix it. Michael -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] potential null dereference in ext/ftp/ftp.c
Hi, Jess Portnoy wrote: clang is indeed a great tool but since it does a lot more than just static analysis. Yeah, it looked like an interesting thing and so I decided to play with it. Incidentally, I discovered later that clang appears to compile PHP 5.3 pretty much flawlessly just now (at least for my particular set of configure options). The scan-build analyser thing I used ran the code through clang before forwarding it on to gcc for the actual compilation. For those cases where one wants source code analysis, especially security oriented, I'd recommend flawfinder [http://www.dwheeler.com/flawfinder]. I'll have a look. Thanks for the tip. I ran it against the PHP 5.2.11 sources and am now sorting through results, patching suggestions may follow :) Heh. If anyone wants to see the output from scan-build that I got, it's at http://mgdm.net/~michael/php-5.3-clang/ along with the notes.txt that I'm filling in as I go along. Michael -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] potential null dereference in ext/ftp/ftp.c
Hey, The thing I like a lot about clang is that it can be used as a drop-in substitute for GCC so you can actual call clang or clang++ instead of executing gcc/g++, see here: http://clang.llvm.org/get_started.html The results you published certainly look interesting :) May the source be with you, Best regards, Jess Portnoy Michael Maclean wrote: Hi, Jess Portnoy wrote: clang is indeed a great tool but since it does a lot more than just static analysis. Yeah, it looked like an interesting thing and so I decided to play with it. Incidentally, I discovered later that clang appears to compile PHP 5.3 pretty much flawlessly just now (at least for my particular set of configure options). The scan-build analyser thing I used ran the code through clang before forwarding it on to gcc for the actual compilation. For those cases where one wants source code analysis, especially security oriented, I'd recommend flawfinder [http://www.dwheeler.com/flawfinder]. I'll have a look. Thanks for the tip. I ran it against the PHP 5.2.11 sources and am now sorting through results, patching suggestions may follow :) Heh. If anyone wants to see the output from scan-build that I got, it's at http://mgdm.net/~michael/php-5.3-clang/ along with the notes.txt that I'm filling in as I go along. Michael -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] potential null dereference in ext/ftp/ftp.c
Jess Portnoy wrote: The thing I like a lot about clang is that it can be used as a drop-in substitute for GCC so you can actual call clang or clang++ instead of executing gcc/g++, see here: Sure, that's how I compiled PHP with it. CC=clang ./configure --enable-all --my-usual=stuff make make test :) -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] potential null dereference in ext/ftp/ftp.c
Jess Portnoy wrote: Hello, clang is indeed a great tool but since it does a lot more than just static analysis. For those cases where one wants source code analysis, especially security oriented, I'd recommend flawfinder [http://www.dwheeler.com/flawfinder]. I find that flawfinder is way too simplistic. It just does a grep for certain strings and spews a canned warning. For example: basic_functions.c:137: [4] (buffer) sprintf: Does not check for buffer overflows. Use snprintf or vsnprintf. basic_functions.c:2778: [4] (buffer) sprintf: Does not check for buffer overflows. Use snprintf or vsnprintf. basic_functions.c:2779: [4] (format) printf: If format strings can be influenced by an attacker, they can be exploited. Use a constant for the format specification. Now, that sounds very scary, until you look at those source code lines: 137: #undef sprintf 2778: PHP_NAMED_FE(sprintf, PHP_FN(user_sprintf), arginfo_sprintf) 2779: PHP_NAMED_FE(printf,PHP_FN(user_printf),arginfo_printf) It is so littered with false positives like this that I don't find it useful. -Rasmus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] potential null dereference in ext/ftp/ftp.c
Hello, I agree that it [potentially] many false positives and the is even addressed in the homepage. While this is common to most static analyzers to some extent and requires going through each find with care while mumbling again with this crap.., I still think it has some value. One person's opinion. May the source be with you, Best regards, Jess Portnoy Rasmus Lerdorf wrote: Jess Portnoy wrote: Hello, clang is indeed a great tool but since it does a lot more than just static analysis. For those cases where one wants source code analysis, especially security oriented, I'd recommend flawfinder [http://www.dwheeler.com/flawfinder]. I find that flawfinder is way too simplistic. It just does a grep for certain strings and spews a canned warning. For example: basic_functions.c:137: [4] (buffer) sprintf: Does not check for buffer overflows. Use snprintf or vsnprintf. basic_functions.c:2778: [4] (buffer) sprintf: Does not check for buffer overflows. Use snprintf or vsnprintf. basic_functions.c:2779: [4] (format) printf: If format strings can be influenced by an attacker, they can be exploited. Use a constant for the format specification. Now, that sounds very scary, until you look at those source code lines: 137: #undef sprintf 2778: PHP_NAMED_FE(sprintf, PHP_FN(user_sprintf), arginfo_sprintf) 2779: PHP_NAMED_FE(printf,PHP_FN(user_printf),arginfo_printf) It is so littered with false positives like this that I don't find it useful. -Rasmus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] [PATCH] potential null dereference in ext/ftp/ftp.c
Hi,
Gwynne pointed me at the clang static analyser earlier on today, and so
I've run it against current PHP_5_3. In the course of messing with it,
it noticed a potential null dereference in ext/ftp - I've attached a
one-liner to fix it.
Michael
Index: ext/ftp/ftp.c
===
--- ext/ftp/ftp.c (revision 291261)
+++ ext/ftp/ftp.c (working copy)
@@ -1699,7 +1699,7 @@
chararg[11];
if (ftp == NULL) {
- goto bail;
+ return PHP_FTP_FAILED;
}
if (!ftp_type(ftp, type)) {
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] potential null dereference in ext/ftp/ftp.c
Michael Maclean wrote: Hi, Gwynne pointed me at the clang static analyser earlier on today, and so I've run it against current PHP_5_3. In the course of messing with it, it noticed a potential null dereference in ext/ftp - I've attached a one-liner to fix it. Thanks, committed. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
