Project

General

Profile

Actions
Copy link

Bug #3070

closed
VJ PA

coverity warnings in protocol detection

Bug #3070: coverity warnings in protocol detection

Added by Victor Julien over 6 years ago. Updated over 6 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Philippe Antoine
Target version:
5.0rc1
Affected Versions:
Effort:
Difficulty:
Label:

Description

** CID 1449368:  Resource leaks  (RESOURCE_LEAK)
/src/app-layer-detect-proto.c: 1483 in AppLayerProtoDetectPMRegisterPattern()

________________________________________________________________________________________________________
*** CID 1449368:  Resource leaks  (RESOURCE_LEAK)
/src/app-layer-detect-proto.c: 1483 in AppLayerProtoDetectPMRegisterPattern()
1477                 PPFunc, pp_min_depth, pp_max_depth);
1478     
1479         goto end;
1480      error:
1481         ret = -1;
1482      end:
>>>     CID 1449368:  Resource leaks  (RESOURCE_LEAK)
>>>     Variable "cd" going out of scope leaks the storage it points to.
1483         SCReturnInt(ret);
1484     }
1485     
1486     /***** Protocol Retrieval *****/
1487     
1488     AppProto AppLayerProtoDetectGetProto(AppLayerProtoDetectThreadCtx *tctx,

** CID 1449367:  Error handling issues  (CHECKED_RETURN)
/src/app-layer-ssl.c: 2856 in RegisterSSLParsers()

________________________________________________________________________________________________________
*** CID 1449367:  Error handling issues  (CHECKED_RETURN)
/src/app-layer-ssl.c: 2856 in RegisterSSLParsers()
2850                                               "443",
2851                                               ALPROTO_TLS,
2852                                               0, 3,
2853                                               STREAM_TOSERVER,
2854                                               SSLProbingParser, NULL);
2855             } else {
>>>     CID 1449367:  Error handling issues  (CHECKED_RETURN)
>>>     Calling "AppLayerProtoDetectPPParseConfPorts" without checking return value (as is done elsewhere 12 out of 13 times).
2856                 AppLayerProtoDetectPPParseConfPorts("tcp", IPPROTO_TCP,
2857                                                     proto_name, ALPROTO_TLS,
2858                                                     0, 3,
2859                                                     SSLProbingParser, NULL);
2860             }
2861         } else {

PA Updated by Philippe Antoine over 6 years ago Actions
Copy link
 #1

The first one is indeed a leak.

The second is a false positive to me.
But we can check the return value and log a warning in case of an error

VJ Updated by Victor Julien over 6 years ago Actions
Copy link
 #2

I think the warning is not a false positive as it is factually correct. It may not be very interesting, but that doesn't make it a FP.

If you check the smb or template parsers, you will see not just a warning but also some other registration happening on failure of AppLayerProtoDetectPPParseConfPorts. I think it makes sense to investigate if we need to do something similar here.

PA Updated by Philippe Antoine over 6 years ago Actions
Copy link
 #3

Sorry for the wording.
Indeed, the warning is factually correct as we call AppLayerProtoDetectPPParseConfPorts without checking its return value (and usually check for it)

Thanks to your SMB pointer, I will propose another PR

Actions
Copy link

Also available in: PDF Atom