[Bro-Dev] [JIRA] (BIT-1422) Lack of Sanity Check in file 'broccoli_intern.i'

[Johanna Amann (JIRA)]

     [ https://bro-tracker.atlassian.net/browse/BIT-1422?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Johanna Amann updated BIT-1422:
-------------------------------
    Resolution: Won't Fix
        Status: Closed  (was: Open)

I am closing this as won't fix.

Broccoli is going away, so we probably do not want to invest any work there. Furthermore, adding error messages on memory exhaustion does not really help - if malloc fails, Bro will crash within a very short time. Just accepting that and crashing on the first 0-pointer malloc returns seems to be preferable to having checks with a error message at every malloc.

> Lack of Sanity Check in file 'broccoli_intern.i'
> ------------------------------------------------
>
>                 Key: BIT-1422
>                 URL: https://bro-tracker.atlassian.net/browse/BIT-1422
>             Project: Bro Issue Tracker
>          Issue Type: Patch
>          Components: broccoli-python
>    Affects Versions: 2.3
>         Environment: Operating System (Linux/Unix/Windows/All)
>            Reporter: Bill Parker
>              Labels: Checking, Sanity
>             Fix For: 2.5
>
>         Attachments: broccoli_intern.i.patch
>
>
> Hello All,
>    In file 'broccoli_intern.i', in directory 'aux/broccoli/bindings/broccoli-python', I found a number of instances where calls to malloc() are made without a corresponding check for a return value of NULL, indicating failure.  The patch file below corrects/addresses this issue:
> --- broccoli_intern.i.orig      2015-06-06 09:02:11.949122426 -0700
> +++ broccoli_intern.i   2015-06-06 09:23:00.187767139 -0700
> @@ -229,6 +229,11 @@
>        case BRO_TYPE_BOOL:
>        case BRO_TYPE_INT: {
>            int64_t* tmp = (int64_t *)malloc(sizeof(int64_t));
> +          if (tmp == NULL) {    /* memory allocation failed... */
> +              PyErr_SetString(PyExc_RuntimeError, "Unable to allocate memory for Bro BOOL/INT");
> +              return 0; /* should we return ENOMEM here instead?  */
> +          }
> +
>                   *tmp = PyInt_AsLong(val);
>            *data = tmp;
>            break;
> @@ -237,6 +242,10 @@
>        case BRO_TYPE_COUNT:
>        case BRO_TYPE_COUNTER: {
>            uint64_t* tmp = (uint64_t *)malloc(sizeof(uint64_t));
> +          if (tmp == NULL) { /* memory allocation failed... */
> +              PyErr_SetString(PyExc_RuntimeError, "Unable to allocate memory for Bro COUNT/COUNTER");
> +              return 0;   /*  should we return ENOMEM here instead? */
> +          }
>                   *tmp = PyInt_AsLong(val);
>            *data = tmp;
>            break;
> @@ -247,6 +256,10 @@
>                return 0;
>  
>            BroAddr* addr = (BroAddr*)malloc(sizeof(BroAddr));
> +          if (addr == NULL) { /* memory allocation failed...  */
> +              PyErr_SetString(PyExc_RuntimeError, "Unable to allocate memory for Bro TYPE_IPADDR");
> +              return 0;       /* should we return ENOMEM here instead?  */
> +          }
>            parseAddrTuple(val, addr);
>            *data = addr;
>            break;
> @@ -256,6 +269,10 @@
>        case BRO_TYPE_TIME:
>        case BRO_TYPE_INTERVAL: {
>            double* tmp = (double *)malloc(sizeof(double));
> +          if (tmp == NULL) {  /* memory allocation failed...  */
> +              PyErr_SetString(PyExc_RuntimeError, "Unable to allocate memory for Bro TYPE DOUBLE/TIME/INTERVAL");
> +              return 0;       /* should we return ENOMEM here instead?  */
> +          }
>                   *tmp = PyFloat_AsDouble(val);
>            *data = tmp;
>            break;
> @@ -269,6 +286,10 @@
>                return 0;
>  
>            str = (BroString *)malloc(sizeof(BroString));
> +          if (str == NULL) {  /* memory allocation failed...  */
> +              PyErr_SetString(PyExc_RuntimeError, "Unable to allocate memory for Bro TYPE_STRING");
> +              return 0;       /* should we return ENOMEM here instead?  */
> +          }
>            str->str_len = strlen(tmp);
>            str->str_val = (uchar*)strdup(tmp);
>            *data = str;
> @@ -282,6 +303,10 @@
>            }
>  
>            int* tmp = (int *)malloc(sizeof(int));
> +          if (tmp == NULL) {  /* memory allocation failed...  */
> +              PyErr_SetString(PyExc_RuntimeError, "Unable to allocate memory for Bro TYPE_ENUM");
> +              return 0;       /* should we return ENOMEM here instead?  */
> +          }
>                   *tmp = PyInt_AsLong(PyTuple_GetItem(val, 0));
>            *data = tmp;
>  
> @@ -300,6 +325,10 @@
>            }
>  
>            BroPort* port = (BroPort *)malloc(sizeof(BroPort));
> +          if (port == NULL) { /* memory allocation failed...  */
> +              PyErr_SetString(PyExc_RuntimeError, "Unable to allocate memory for Bro TYPE_PORT");
> +              return 0;       /* should we return ENOMEM here instead?  */
> +          }
>            port->port_num = PyInt_AsLong(PyTuple_GetItem(val, 0));
>            port->port_proto = PyInt_AsLong(PyTuple_GetItem(val, 1));
>            *data = port;
> @@ -316,6 +345,10 @@
>                return 0;
>  
>            BroSubnet* subnet = (BroSubnet *)malloc(sizeof(BroSubnet));
> +          if (subnet == NULL) { /* memory allocation failed...  */
> +              PyErr_SetString(PyExc_RuntimeError, "Unable to allocate memory for Bro TYPE_SUBNET");
> +              return 0;
> +          }
>  
>            parseAddrTuple(addr, &subnet->sn_net);
>  
> I am attaching the patch file to this bug report...
> Bill Parker (wp02855 at gmail dot com)



--
This message was sent by Atlassian JIRA
(v7.0.0-OD-08-002#70107)