cgltf_parse_file can crash with custom file_open, should not call memory_free

[pixeljetstream]

cgltf_parse_file crashes when the parsing fails with a custom read/release callback in use. A pointer acquired from file_read cannot be passed to memory_free, but that is done here.

    void* file_data = NULL;
    cgltf_size file_size = 0;
    cgltf_result result = file_read(&options->memory, &options->file, path, &file_size, &file_data);
    if (result != cgltf_result_success)
    {
        return result;
    }

    result = cgltf_parse(options, file_data, file_size, out_data);

    if (result != cgltf_result_success)
    {
                // BUG? this should be file_release as the file_data was acquired by file_read
        memory_free(options->memory.user_data, file_data); 
        return result;
    }

[jkuhlmann]

Can you confirm that PR #160 would fix this?

[NBickford-NV]

To help out with this, I looked through all the occurrences of options->file.release and options->memory.free in cgltf.h. Assuming file.release and memory.free are not intended to be interchangeable (based on the issue #94 discussion here), then there may be two issues:

• cgltf_parse_file() gets a file_data pointer using file.read above. But this pointer is later passed to either memory_free here if the parse fails, or passed to file.release in cgltf_free() here. (Looks like Johannes posted a fix right as I was writing this - I'll test it out!)
• When reading buffers, buffers stored in files are read using file.read here, but Base64 buffers are decoded to a buffer allocated using memory.alloc here. Both of these appear to be released using file.release in cgltf_free() here (there's a special case for the .glb binary section which gets handled correctly!)

Also - thanks very much; I'll test out the PR and hopefully it should fix the cgltf_parse_file() issue!

[jkuhlmann]

Funny that this all comes up now. Your second bullet point is the same as #158, is that correct?

[NBickford-NV]

Oh, yep, looks like that is indeed the same issue!

Also, I can confirm that PR #160 fixes the cgltf_parse_file() issue with custom file callbacks and invalid glTF files - thank you, Johannes!

[jkuhlmann]

I think in that case we don't have much of a choice except for tracking where the data comes from. I've added another change to #160. Does that help?

[pixeljetstream]

we didn't run into #158 but your fixes looks good