AcademySoftwareFoundation / AcademySoftwareFoundation/openexr
Unsafe pattern in OpenEXRCore with custom read/write fns
- Dominant language
- C
- Stars
- 1.8k
- Forks
- 700
- Avg merge
- 3d 12h
- Merged PRs (30d)
- 29
Description
This one took me some time to debug, and an error check would have helped a ton. The scenario:
I am reading exr files from a custom CMS, so I have written a custom readfn. I am writing to the standard file system, so I have not specified a custom writefn.
The comment below describe the issue.
```c
exr_context_initializer_t exrInit = EXR_DEFAULT_CONTEXT_INITIALIZER;
// exrInit.read_fn = exr_AssetRead_Func;
// exrInit.user_data = (void*) customReadData; // data for exr_AssetRead_Func.
exrInit.read_fn = NULL; // If read is set, and write is not, things fail in write because user_data doesn't get allocated
// and then when we try to set fh->fd = -1 that's a seg fault
exrInit.write_fn = NULL; // Use the default file system write
```
Reading works fine with a custom read_fn and a custom NULL write_fn, which misled me into thinking that was a legitmate pattern. My thought is that either
- the default read should not write to fh->fd but some safe storage, or,
- if the exr_start_write is called, and read_fn is not NULL and write_fn is NULL, then an error code should be returned, or
- there should be read_user_data and write_user_data.
- minimally we should document the danger zone
The immediate remedy of course is for me to supply the write function, but I think we can do something in the Core lib...
Contributor guide
Research direction
Start by tracing the OpenEXRCore handling of exr_context_initializer_t, custom read_fn/write_fn combinations, and the exr_start_write entry point. Reproduce the initializer shown with a custom read function and NULL write function, then determine the expected safe behavior or error contract; done means the combination no longer reaches an unsafe state and is covered by an appropriate test or documentation.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c
- Domain
- computer-graphics
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100