Return values - Printable Version +- CodeIgniter Forums (https://forum.codeigniter.com) +-- Forum: Archived Discussions (https://forum.codeigniter.com/forumdisplay.php?fid=20) +--- Forum: Archived Development & Programming (https://forum.codeigniter.com/forumdisplay.php?fid=23) +--- Thread: Return values (/showthread.php?tid=12045) Pages:
1
2
|
Return values - El Forum - 10-03-2008 [eluser]Aethedor[/eluser] If done some (quick) code auditing and seen that on several places, the return values of called functions are not handled properly. Specially towards error values. Below is a list of my findings of the files in /libararies. libraries/Email.php attach(), line 368 specs say @return string, function returns NULL validate_email(), line 684 function returns FALSE on error, but NULL on success. should be TRUE. function is called on several places in class, but return value is not checked. _build_message(), line 959 function returns FALSE on error, but NULL on success. should be TRUE. function is called on several places in class, but return value is not checked. _spool_email(), line 1370 function is called on line 1327, but return value is not checked. _smtp_connect(), line 1532 function returns FALSE on error, but NULL on success. should be TRUE. _send_command(), line 1560 function is called on several places in class, but return value is not checked. function is called on line 1476, but return value is not checked. _smtp_authenticate(), line 1625 function is called on line 1477, but return value is not checked. _send_data(), line 1679 function is called on several places in class, but return value is not checked. libraries/Ftp.php delete_dir(), line 370 function should check return value of delete_dir on line 390 and return FALSE on error mirror(), line 482 return value of mirror() (line 507) not checked for errors. return FALSE on error. return value of upload() (line 515) not checked for errors. return FALSE on error. libraries/Hook.php _call_hook(), line 91 return value of _run_hook() (line 102, 107) not checked for errors. return FALSE on error. specs say @return mixed, function returns boolean. _run_hook(), line 124 return value on line 140 is not a boolean libraries/Image_lib.php initialize(), line 128 specs say @return void, function returns boolean. image_process_netpbm(), line 628 return value of copy() (line 696) and unlink() (line 697) not checked for errors, return FALSE on error. size_calculator(), line 1383 return value NULL should be FALSE (line 1386). libraries/Language.php load(), line 52 specs say @return void, function returns TRUE and string not defined variable used ($lang) on line 86 libraries/Loader.php library(), line 74 function returns FALSE on error, but NULL on success. should be TRUE. (specs even say @return void) database(), line 194 function should return object, but at the end of the function it returns NULL. dbutil(), line 231 specs say @return string, function returns NULL. dbforge(), line 261 specs say @return string, function returns NULL. view(), line 298 specs say @return void, function returns !void; _ci_load(), line 630 specs say @return void, function returns string (line 720). _ci_autoloader(), 889, specs say @return void, function returns FALSE on error (line 895). libraries/Output.php _write_cache(), line 281 return values of flock and fwrite not checked for errors (line 307). _display_cache(), line 324 specs say @return void, function retuns boolean. return value of flock not checked for errors, line 350. libraries/URI.php _fetch_uri_string(), line 60 specs say @return string, function returns NULL libraries/User_agent.php _compile_data(), line 133 specs say @return bool, function returns NULL call to function should check for error code, line 70 libraries/Session.php sess_read(), line 183 specs say @return void, function returns boolean. libraries/Validation.php set_select(), line 648 function returns NULL at the end, not ''. set_radio(), line 674 function returns NULL at the end, not ''. libraries/Zip.php read_dir(), line 236, returns NULL on opendir() error. archive(), line 296 return value of flock() and fwrite() not checked. Return values - El Forum - 10-06-2008 [eluser]Aethedor[/eluser] Any of the CI developers who want to respond to this? Return values - El Forum - 10-07-2008 [eluser]Aethedor[/eluser] Is the CI projectteam going to take security serious or not? Otherwise I will stop trying to improve the quality of the code. Return values - El Forum - 10-07-2008 [eluser]xwero[/eluser] Most your remarks have to do with a wrong code documentation, right? For the Language library i can say that the $lang variable is in the language file, so if it's defined in the library developers will not know there is something wrong with the language file they want to load. For the Validation library when will the methods return NULL? You should have some patience the EL developers are hard at work to get their bread and butter application out of the door which is already late. Return values - El Forum - 10-07-2008 [eluser]Aethedor[/eluser] [quote author="xwero" date="1223389013"]Most your remarks have to do with a wrong code documentation, right?[/quote] No, it's about inconsistancy in the return values. It's about not getting back from a function what you expect, which can result in undesired code behaviour, which can result into a security problem. Quote:For the Language library i can say that the $lang variable is in the language file, so if it's defined in the library developers will not know there is something wrong with the language file they want to load.Should the function not check $lang is an array after the language file is included and raise an error if it's not? It now looks a bit hackish to me. These kind of checks make CodeIgniter more robust. Quote:For the Validation library when will the methods return NULL?When the 2 if-statements result into a false. The function just ends without a return, therefor returning null (void). Quote:You should have some patience the EL developers are hard at work to get their bread and butter application out of the door which is already late.Oke, but after 4 days I expected at least a "we're busy, we'll get back to you" kind of reply. The forum now looks pretty much dead to me... Return values - El Forum - 10-07-2008 [eluser]Aethedor[/eluser] There seems to be something wrong with this forum. It's the second time I pressed the [Submit Post] button while my post got lost.... Return values - El Forum - 10-07-2008 [eluser]xwero[/eluser] You are right about an is_array check for the $lang variable but the isset check is to see if there is a $lang variable present so it has to come before the array check. Return values - El Forum - 10-07-2008 [eluser]Derek Allard[/eluser] Aethedor, I'd repeat the call for patience please. Which of the issues that you've identified do you feel are security issues, and why? I assure you that we take security very seriously. I recognize that some of the code comments you've identified are wrong (thanks) but I have to move these to a lower priority then security. Return values - El Forum - 10-07-2008 [eluser]Aethedor[/eluser] All the issues are inconsistancy in function declaration and usage of return values. This can lead to incorrect error checking when using these functions. Incorrect checking for error values (or not checking at ) when calling functions is a major cause of security issues. I'm sure there are a lot of excuses why this isn't a security thing, but I don't care about those. I've seen many bad/non-error checking result into big security issues. The only thing I'm gonna do is share my experiences and knowledge about IT security with you to help you improve CodeIgniter's code quality. It's up to you what to do with it. Return values - El Forum - 10-07-2008 [eluser]Aethedor[/eluser] Seriously guys, fix your forum. This is the third time I enter a reply, press submit and all I end up with is an empty inputform and no reply submitted..... I'm not going to enter every post on this forum twice..... |