splone / splonebox-core

splonebox - open source network assessment tool with focus on modularity
http://splonebox.io
GNU Affero General Public License v3.0
24 stars 7 forks source link

faulty/missing check in dispatch.c #24

Open bontric opened 8 years ago

bontric commented 8 years ago

meta is a pointer to a message_params_object stored in a message_object and therefore can't be NULL.

// dispatch.c:169
 meta = &request->params.obj[0].data.params;
// ...
// dispatch.c:176
if (!meta) {
  error_set(api_error, API_ERROR_TYPE_VALIDATION,
         "Error dispatching run API request. meta params is NULL");
  return (-1);
  }

meta->obj could potentially be NULL though and there is no check for it!

mkind commented 8 years ago

How can this happen? Is there a test, that covers this situation?

bontric commented 8 years ago

@mkind The unit test for 'handle_run' addresses this

How can this happen?

There was no unit test ..

mkind commented 8 years ago

Ah.. I see. Quoting unpack.c:

  /* if array is empty return success */
  if (obj->via.array.size <= 0) {
    params->obj = NULL;
    params->size = 0;
    return (0);
  }
mkind commented 8 years ago

As far as I see, iff meta->obj is NULL, then meta->size is set to zero. Since the size is tested before accessing obj there should be no problem?

  if (meta->size != 5) {
    error_set(api_error, API_ERROR_TYPE_VALIDATION,
        "Error dispatching register API request. Invalid meta params size");
    return (-1);
  }
bontric commented 8 years ago

:+1:

mkind commented 8 years ago

seem like meta->obj can not be null

Well, yes it can be null as mentioned above. If so, the size is set to zero which is checked.

bontric commented 8 years ago

Well, yes it can be null as mentioned above

Didn't see that comment..

mkind commented 8 years ago

Is it ok to close issue?

bontric commented 8 years ago

The check still needs to be removed