AcademySoftwareFoundation / AcademySoftwareFoundation/OpenColorIO
Asan container overflow when reading configuration file
- Dominant language
- C++
- Stars
- 2.1k
- Forks
- 503
- PR merge metrics
- No merged PRs in 30d
Description
I'm using the source for v2.4.0 and I'm seeing a container overflow when building with address sanitizer enabled:
```
#0 0x0000000311ca31a8 in __asan::AsanDie ()
#1 0x0000000311cbe7a0 in __sanitizer::Die ()
#2 0x0000000311ca109c in __asan::ScopedInErrorReport::~ScopedInErrorReport ()
#3 0x0000000311ca03d8 in __asan::ReportGenericError ()
#4 0x0000000311c95d10 in __asan_memcpy ()
#5 0x000000010cb6b4b0 in OpenColorIO_v2_4::ColorSpace::getAllocationVars at /build/mb/git/adobe/thirdparty/OpenColorIO/src/OpenColorIO/ColorSpace.cpp:305
#6 0x000000010ccb5f8c in OpenColorIO_v2_4::(anonymous namespace)::save at build/mb/git/adobe/thirdparty/OpenColorIO/src/OpenColorIO/OCIOYaml.cpp:3356
#7 0x000000010cc5826c in OpenColorIO_v2_4::(anonymous namespace)::save at build/mb/git/adobe/thirdparty/OpenColorIO/src/OpenColorIO/OCIOYaml.cpp:5097
#8 0x000000010cc5367c in OpenColorIO_v2_4::OCIOYaml::Write at build/mb/git/adobe/thirdparty/OpenColorIO/src/OpenColorIO/OCIOYaml.cpp:5151
#9 0x000000010cbb81d0 in OpenColorIO_v2_4::Config::serialize at build/mb/git/adobe/thirdparty/OpenColorIO/src/OpenColorIO/Config.cpp:4937
#10 0x000000010cbb8a84 in OpenColorIO_v2_4::Config::getCacheID at build/mb/git/adobe/thirdparty/OpenColorIO/src/OpenColorIO/Config.cpp:4882
#11 0x000000010cbb8564 in OpenColorIO_v2_4::Config::getCacheID at build/mb/git/adobe/thirdparty/OpenColorIO/src/OpenColorIO/Config.cpp:4861
#12 0x000000010cb43100 in OCIOW::OCIOConfiguration::OCIOConfiguration at MediaCore/OCIOModules/OCIOWrapper/Src/OCIOConfigurationFileManager.cpp:238
#13 0x000000010cb47a18 in OCIOW::OCIOConfiguration::ValidateOCIOConfigurationFile [inlined] at MediaCore/OCIOModules/OCIOWrapper/Src/OCIOConfigurationFileManager.cpp:220
...
```
This is reported as a container overflow issue [here](https://github.com/AcademySoftwareFoundation/OpenColorIO/blob/d807b380d24d826bd97ca1bb356ff8b83c006563/src/OpenColorIO/OCIOYaml.cpp#L3355-L3356)
With all the manual memory copying happening into that vector, I'm not surprised an overflow happens. Instead it should be:
```
std::vector allocationvars;
allocationvars.resize(cs->getAllocationNumVars());
```
It's important that the vector's capacity can handle the size of memory that's going to be copied into it by the manual memcpy. Otherwise this is always at risk of a container overflow if the vector's capacity is different than the size (which can happen if the vector starts as 0 elements and a memcpy tries to copy in 3, like what I encountered in my container overflow debug case locally.
It looks to me like there is a similar risk [here](https://github.com/AcademySoftwareFoundation/OpenColorIO/blob/d807b380d24d826bd97ca1bb356ff8b83c006563/src/OpenColorIO/OCIOYaml.cpp#L503) as well.
Contributor guide
Research direction
Inspect src/OpenColorIO/OCIOYaml.cpp around lines 3355-3356 and 503, along with ColorSpace::getAllocationVars in src/OpenColorIO/ColorSpace.cpp. Reproduce with an AddressSanitizer build and a configuration that triggers serialization. Done means the allocation-variable vectors have sufficient size before copying and both reported locations are covered without an ASan container-overflow report.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- computer-graphics
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100