[Openvas-commits] r1153 - in trunk/openvas-libnasl: . nasl

[scm-commit@wald.intevation.org]

Author: jan
Date: 2008-08-21 14:35:04 +0200 (Thu, 21 Aug 2008)
New Revision: 1153

Modified:
   trunk/openvas-libnasl/ChangeLog
   trunk/openvas-libnasl/nasl/nasl_socket.c
Log:
* nasl/nasl_socket.c (nasl_close_socket): Added some thoughts from
the mailing list as comments so they are directly present they eye
of a developer turns here (again).



Modified: trunk/openvas-libnasl/ChangeLog
===================================================================
--- trunk/openvas-libnasl/ChangeLog	2008-08-21 12:27:30 UTC (rev 1152)
+++ trunk/openvas-libnasl/ChangeLog	2008-08-21 12:35:04 UTC (rev 1153)
@@ -1,3 +1,9 @@
+2008-08-21  Jan-Oliver Wagner <jan-oliver.wagner at intevation.de>
+
+	* nasl/nasl_socket.c (nasl_close_socket): Added some thoughts from
+	the mailing list as comments so they are directly present they eye
+	of a developer turns here (again).
+
 2008-08-06   Chandrashekhar B <bchandra at secpod.com>
 
 	* nasl/nasl_socket.c (nasl_close_socket): Fixed an issue with UDP socket close.

Modified: trunk/openvas-libnasl/nasl/nasl_socket.c
===================================================================
--- trunk/openvas-libnasl/nasl/nasl_socket.c	2008-08-21 12:27:30 UTC (rev 1152)
+++ trunk/openvas-libnasl/nasl/nasl_socket.c	2008-08-21 12:35:04 UTC (rev 1153)
@@ -612,6 +612,29 @@
  int e;
  
  soc = get_int_var_by_num(lexic, 0, -1);
+ /* XXX: These are thoughts expressed on the openvas-devel mailing list 2008-08-06:
+  *
+  * nasl_close_socket seems to be the only place in nasl/nasl_socket.c where the
+  * value of the socket filedescriptor is checked in this way.  That in itself is
+  * strange.  Why only there?  Also, why can't the socket fd be less than 4?  I
+  * could sort of understand 3 (0, 1, 2 are already taken by the standard
+  * streams) but 4? Does the openvas server and/or the NASL interpreter guarantee
+  * that at least one other file is open?
+  *
+  * My guess is that the check is there to prevent NASL scripts from closing file
+  * descriptors needed by openvas/NASL which includes the ones it uses for
+  * accessing the knowledgebase.  If that's the case, then the test has too much
+  * knowledge of the circumstances under which the NASL interpreter runs.  It
+  * should be moved to a separate function whose behavior can be influenced by
+  * the program embedding the NASL interpreter.  Other functions should probably
+  * also check the descriptors.
+  *
+  * I also wonder whether the original code (disallowing any file descriptor <= 4) 
+  * actually was correct and the real defect is that open_sock_udp actually
+  * returned 4.  Under which circumstances does it actually do that?  In my brief
+  * tests with the stand-alone nasl interpreter the smallest number it returned
+  * was 5.
+ */
  if(soc < 4)
 	{
  	 nasl_perror(lexic, "close(): invalid argument\n");