Hi,

error handling in the NASL interpreter and reporting of such errors to 
openvasd and back to the user is not ideal and it doesn't seem difficult to 
improve.  Attached is a brief analysis and some suggestions for improvements.
What do you think?

   Bernhard

-- 
Bernhard Herzog                              Intevation GmbH, Osnabrück
Amtsgericht Osnabrück, HR B 18998             http://www.intevation.de/
Geschäftsführer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner
Improving Error Reporting for OpenVAS NVTs
==========================================
2008-04-22, Bernhard Herzog <[EMAIL PROTECTED]>


Currently, it is not easily possible for an OpenVAS user to determine
errors that occurred in NVTs.  For instance, the result of a test run
does not distinguish between an NVT that didn't report any errors or
security issues and an NVT that could not be run because of unexpected
errors, such as missing .inc files.

A related problem is that the insufficient error reporting can lead to
invalid plugin-descriptions sent by the openvasd to the client [1].


All of the problems discussed here have to do with the NASL interpreter.
NASL Scripts are run with this libnasl function:

    int execute_nasl_script(struct arglist * script_infos,
                            const char* name, 
                            const char * cache_dir, 
                            int mode)

The return value indicates whether the call was successful (>=0) or not
(<0).

Thus, execute_nasl_script can indicate to its caller when something went
wrong (such as a missing .inc file), but it does not do so in many cases.


Cases that need to be fixed:


Parsing Errors
--------------

execute_nasl_script indirectly calls init_nasl_ctx and naslparse to open
the .nasl-file and parse it.  Both function return error codes which are
correctly propagated to the caller of execute_nasl_script. 

naslparse also handles the NASL include statements.  It reads the
include file and parses it, but, of course, does not yet execute it.
The naslparse function does not propagate errors resulting from failed
includes, though.  It prints error messages to stderr, but it does not
return an error code if an include file cannot be read or has syntax
errors.

This is not necessarily a defect.  The include statement may be inside a
conditional, and thus it may not actually be executed at run-time, in
which case a missing or syntactically incorrect include file would be no
problem.  In most NASL-files, include statements are at the top-level,
and therefore not inside of conditionals or other constructs.  However,
at least one file has a conditional include
(openvas-plugins/scripts/dameware_mini_remote_control_disclosure.nasl)


Solutions:

One solution would be to create a new node type for failed includes that
is inserted into the parse tree where the parse tree of the include file
would have been inserted.  The interpreter would then treat it as a
runtime error if it is actually executed.

An alternative would be to simply propagate the parse error up the call
stack so that eventually execute_nasl_script returns with an error code.
This solution has the disadvantage, that conditional includes may result
in parse errors for include files that will not actually be executed.

The first solutions is preferable because it doesn't change the
semantics of NASL.


Runtime Errors
--------------

Runtime Errors Determined by the Interpreter
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Examples of this kind of error:

 - Calling an unknown function

 - Passing the wrong type of parameter to a function or operator

 - Calling a function with too few parameters


The interpreter generally only reports such errors on stderr.  They
don't affect the execution.

Solutions:

At the very least, the fact that errors have occurred should be
recorded.  This could be done with an additional field in the struct
tree_cell, error_flag, which is set whenever an error occurred.  This
fact should be reported to the openvasd in a way similar to a security
warning, so that the OpenVAS client will see the error in the report.
Also, execute_nasl_script should return an error code indicating that
something went wrong.

Alternatively, we could make the NASL interpreter return immediately if
an error occurs.  There are already some provisions in the code for
this.  For instance, openvas-libnasl/nasl/exec.c may be compiled with
STOP_AT_FIRST_ERROR #defined, in which case the interpreter will exit
when an error occurs (in most cases anyway).  However, this would change
the semantics of NASL and might therefore affect existing NASL scripts
in hard to predict ways.

The first solutions is preferable because it doesn't change the
semantics of NASL.


Runtime Errors found by NASL Code
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

If errors are found by NASL Code, the code should report it with one of
the security functions (security_warning, security_note, security_hole),
so that the user sees it.  We should probably add another function that
indicates that the script could not be run because errors were
encountered.




Footnotes
---------

[1] Thread "Broken .desc handling and broken error handling" on openvas-devel:
http://lists.wald.intevation.org/pipermail/openvas-devel/2008-April/000649.html

Attachment: pgpuddP8XWh3X.pgp
Description: PGP signature

_______________________________________________
Openvas-devel mailing list
[email protected]
http://lists.wald.intevation.org/mailman/listinfo/openvas-devel

Reply via email to