AcademySoftwareFoundation / AcademySoftwareFoundation/openexr

Unsafe pattern in OpenEXRCore with custom read/write fns

Open
#1,432 0 comments 0 reactions 0 assignees View on GitHub
needs attention
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.