Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential memory leak in the bson_array.c file #33

Closed
NicoleYarroch opened this issue May 24, 2019 · 1 comment · Fixed by #55
Closed

Potential memory leak in the bson_array.c file #33

NicoleYarroch opened this issue May 24, 2019 · 1 comment · Fixed by #55
Labels

Comments

@NicoleYarroch
Copy link
Contributor

NicoleYarroch commented May 24, 2019

The iOS library generates a "potential memory leak" warning in the size_t bson_array_from_bytes_len() method. This happens in the else statement of the TYPE_ARRAY case:

case TYPE_ARRAY: {
    BsonArray subArray;
    ret = bson_array_from_bytes_len(&subArray, current, remainBytes);
    if (ret > 0) {
        bson_array_add_array(&array, &subArray);
        current += ret;
        remainBytes -= ret;
    } else {
        parseError = true; // <- Potential memory leak warning generated by Xcode
    }
    break;
}

Step 1:
1
Steps 2 & 11:
2, 11
Steps 3, 4, 12, & 13:
3, 4, 12, 13
Steps 5 & 14:
5, 14
Steps 6 & 15:
6, 15
Steps 7, 8, 20, & 21:
7, 8, 20, 21
Step 9:
9
Steps 10 & 22:
10, 22
Steps 16 & 19:
16, 19
Steps 17 & 18:
17, 18
Steps 23 & 24:
23, 24

OS & Version Information
  • bson_c_lib Version: 1.2.0
  • SDL iOS Version: 6.2.3

The full method:

size_t bson_array_from_bytes_len(BsonArray *output, const uint8_t *data, size_t dataSize) {
  const uint8_t *current = data;
  size_t remainBytes = dataSize;
  int32_t size = 0;
  bool parseError = false;
  size_t ret;

  if (output == NULL || data == NULL || dataSize < SIZE_INT32) {
    return 0;
  }
  size = read_int32_le((uint8_t **)&current);
  remainBytes -= SIZE_INT32;
  if (size > dataSize) {
    printf("Unexpected array length %i, data is only %i bytes\n", (int)size, (int)dataSize);
    return 0;
  }

  if (remainBytes < 1) {
    return 0;
  }
  uint8_t type = *current;
  current += 1;
  remainBytes -= 1;

  BsonArray array;
  bson_array_initialize(&array, 10);

  while (type != DOCUMENT_END) {
    char *key = NULL;
    ret = read_string_len(&key, &current, &remainBytes);
    if (ret == 0) {
      parseError = true;
      break;
    }

    switch ((element_type)type) {
      case TYPE_DOCUMENT: {
        BsonObject obj;
        ret = bson_object_from_bytes_len(&obj, current, remainBytes);
        if (ret > 0) {
          bson_array_add_object(&array, &obj);
          current += ret;
          remainBytes -= ret;
        } else {
          parseError = true;
        }
        break;
      }
      case TYPE_ARRAY: {
        BsonArray subArray;
        ret = bson_array_from_bytes_len(&subArray, current, remainBytes);
        if (ret > 0) {
          bson_array_add_array(&array, &subArray);
          current += ret;
          remainBytes -= ret;
        } else {
          parseError = true;
        }
        break;
      }
      case TYPE_INT32:
        if (remainBytes >= SIZE_INT32) {
          int32_t value = read_int32_le((uint8_t **)&current);
          bson_array_add_int32(&array, value);
          remainBytes -= SIZE_INT32;
        } else {
          parseError = true;
        }
        break;
      case TYPE_INT64:
        if (remainBytes >= SIZE_INT64) {
          int64_t value = read_int64_le((uint8_t **)&current);
          bson_array_add_int64(&array, value);
          remainBytes -= SIZE_INT64;
        } else {
          parseError = true;
        }
        break;
      case TYPE_STRING:
        // Buffer length is read first
        if (remainBytes >= SIZE_INT32) {
          int32_t bufferLength = read_int32_le((uint8_t **)&current);
          remainBytes -= SIZE_INT32;

          if (bufferLength <= remainBytes) {
            char *stringVal = byte_array_to_bson_string((uint8_t*)current, (size_t)bufferLength - 1);
            bson_array_add_string(&array, stringVal);
            free(stringVal);
            current += bufferLength;
            remainBytes -= bufferLength;
          } else {
            parseError = true;
          }
        } else {
          parseError = true;
        }
        break;
      case TYPE_DOUBLE:
        if (remainBytes >= SIZE_DOUBLE) {
          double value = read_double_le((uint8_t **)&current);
          bson_array_add_double(&array, value);
          remainBytes -= SIZE_DOUBLE;
        } else {
          parseError = true;
        }
        break;
      case TYPE_BOOLEAN:
        if (remainBytes >= 1) {
          uint8_t value = *current;
          bson_array_add_bool(&array, value);
          current += 1;
          remainBytes -= 1;
        } else {
          parseError = true;
        }
        break;
      default: {
        printf("Unrecognized BSON type: %i\n", type);
        parseError = true;
      }
    }
    free(key);

    if (parseError) {
      break;
    }

    if (remainBytes >= 1) {
      type = *current;
      current += 1;
      remainBytes -= 1;
    } else {
      parseError = true;
      break;
    }
  }

  if (parseError) {
    bson_array_deinitialize(&array);
    return 0;
  }

  if (data + size != current) {
    printf("Unexpected parsed array size. Expected %i, got %i\n", (int)size, (int)(current - data));
  }
  *output = array;
  return dataSize - remainBytes;
}
@jacobkeeler
Copy link
Contributor

Closing with the merge of #55

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants