Investigation on inappropriate use of panic-recover paradigm

[Mzack9999]

Nuclei version:

main|dev

Current Behavior:

Almost all protocols executions are wrapped within panic-recover, which in fact turns most of runtime fatal crashes into runtime silent errors never reported, and hence fixed, if verbose mode is not specified. This also has unexpected effects if for any reason the panic is triggered within any external callback function in caller code.

Expected Behavior:

The panic-recover paradigm should be either optional unless scoped properly in those code paths that should keep running even on fatal errors (eg. third party library panicking on internal "fatal" errors which are not fatal for the caller execution). The task is about investigating if a better scoping is possible and more effective.

[tarunKoyalwar]

@Mzack9999 , how about allowing to define panic-recover behaviour via env variables by abstracting panic handler in a seperate package ( probably utils)

func PanicHandler(OnError func(error)) {
    if r = recover(); r != nil {
       if os.Getenv("RECOVER_PANIC") = "true" {
              if OnError != nil {
                   OnError(errorutil.New("panic", "recovered panic: %v %v",r, debug.Stack()))
              }
      } else {
         panic(r)
     }
   }
}

^ or something similar/better to pass more context/metadata related to panic

and this can be used in nuclei and other tools

   func UnsafeFunction()  {
       defer PanicHandler()
  }

• while panic are definetly helpful to monitor and debug the cause , it might make tool unusable if we fail to catch those and some are intentional ( for example: goja recommends all errors to be passed by panic due to its design <- see nucleijs utils ). we should probably try to strike a good balance between the two ( as you mention handle internal panics of 3rd party libraries by default and maybe env can be used as switch to control overall behaviour ?? )

cc: @Ice3man543

[Mzack9999]

Recovered panics tend to be ignored over time, while this might work for web applications that should be fault tolerant by nature (ex. http servers), within CLI apps, unless there is a specific design pattern (for example goja as you mentioned), I believe each panic should be investigated and fixed.

I think the ENV variable approach might be a good starting point.

[tarunKoyalwar]

to better identify & locate & fix errors that might be hidden due to panic recovery . removed all recover statements unless its specifically used for thirdparty ( goja or go-rod) #4966