Well, I'm also hedging my bets that just because I can guess that an allocation can go bad, that doesn't mean it's the only kind of problem that can result from nonsensical image dimensions. Who knows what other places could have an arithmetic overflow or other issues, causing cascading errors long before the allocation. Once we're reasonably confident that the file we're reading is likely to be corrupted or sabotaged, we should probably error and stop dealing with that image at the earliest opportunity.
> On Dec 4, 2021, at 2:55 PM, Anders Langlands <anderslangla...@gmail.com> > wrote: > > Yep that makes sense. Personally catching the issue at the point you’re about > to do the unreasonable allocation seems the most “right” to me, but yeah with > overcommit etc it’s not trivial to decide when to fail in that framework > either. > > > On Sun, 5 Dec 2021 at 11:25, Larry Gritz <l...@larrygritz.com > <mailto:l...@larrygritz.com>> wrote: > Anders, I think you are describing exactly the behavior I'm proposing (see > the error message quoted below when I replied to Thomas). > > I'm thinking runtime option rather than a build option as you suggested, > because not all downstream users have the ability to build from scratch; > often, they get whatever built libopenimageio is on their system. > > The attack vector is presumed to be a file constructed to have a header that > claims to have an outrageous number of channels (or simply resolution), > regardless of whether the data is actually present in the file, thus causing > the app or OIIO internals to make allocations that could fail, or potentially > crash the app, or force into a swap and poor performance of the rest of the > system. > > Another potential choice is to push all responsibility for sanity checks to > place that does the allocation. I could fix the many places in OIIO where I > allocate (maybe? would I really find them all?), but that seems like this > would still involve the same guessing about what a reasonable limit is, and > the logic would be duplicated in many places. Not to mention that I'd need to > trust that everybody downstream is also implementing those guards reasonably. > So I think there is benefit in catching this kind of thing as far upstream as > possible, when headers are being read, and just treat them as corrupted files. > > >> On Dec 4, 2021, at 2:11 PM, Anders Langlands <anderslangla...@gmail.com >> <mailto:anderslangla...@gmail.com>> wrote: >> >> Would it not be better as a build option (on by default say) that would exit >> with an error message when trying to open a questionable file? That way you >> get safety by default with an easy path to follow for those who need it. >> >> This is assuming of course that it’s not easier just to guard against >> whatever the knock-on effects of large numbers of channels are. Like what’s >> the actual attack vector here? > > > >> On Dec 4, 2021, at 12:30 PM, Larry Gritz <l...@larrygritz.com >> <mailto:l...@larrygritz.com>> wrote: >> >> I'm thinking something like this: >> >> $ oiiotool bad.tif -o out.tif >> oiiotool ERROR: read : "bad.tif": Uncompressed image size 52127 MB exceeds >> "limit:imagesize_MB" = 32768. Possible corrupt input? >> If you're sure this is a valid file, raise the OIIO global attribute >> "limit:imagesize_MB". >> >> >>> On Dec 4, 2021, at 11:07 AM, Thomas Mansencal <thomas.mansen...@gmail.com >>> <mailto:thomas.mansen...@gmail.com>> wrote: >>> >>> > it's on you to warn OIIO that this is the kind of legit data you are >>> > expecting. >>> >>> Are you considering to warn the user beforehand though in case he tries to >>> pass such image? Be annoying to have the library starting to silently fail >>> all of sudden. >>> >>> On Sun, 5 Dec 2021 at 7:21 AM, Larry Gritz <l...@larrygritz.com >>> <mailto:l...@larrygritz.com>> wrote: >>> For what I'm thinking about, one can raise the limit with a call to the >>> library (or have no limit at all). So if you're dealing with a research >>> renderer with one channel per nm of spectrum, maybe it's on you to warn >>> OIIO that this is the kind of legit data you are expecting. >>> >>> I'm hoping to find a reasonable default where the likelihood is very high >>> that more than that number of channels indicates bad input, and only >>> advanced users who know that they are in this situation ever need to know >>> about and alter the option. >>> >>> 10 is too low -- although most people write AOVs to different files or at >>> least different parts of a multi-image file, I know some users have dozens >>> of channels crammed into one part/subimage. >>> >>> 1000000 is too high, that surely indicates a broken or malicious file. >>> >>> Somewhere in the middle is the right limit. >>> >>> And similarly, I'm also thinking about a maximum memory size for a single >>> flat 2D image as a sanity check to avoid being "tricked" into a memory >>> allocation that will fail (again, a limit you can override if you know such >>> images are coming). I'm thinking 4GB as the default. Does that sound >>> reasonable? Too low? What about 32GB (that's 64k x 64k x 4 chan x >>> sizeof(half))? >>> >>> >>>> On Dec 4, 2021, at 9:55 AM, Thomas Mansencal <thomas.mansen...@gmail.com >>>> <mailto:thomas.mansen...@gmail.com>> wrote: >>>> >>>> Hi Larry, >>>> >>>> With a spectral renderer, assuming storage every nanometers, 400+ >>>> irradiance channels are totally plausible. This is more a research case >>>> though, e.g. Mitsuba 2, but it is possible to see where this goes as soon >>>> as you start combining that with AOVs, e.g. spectral direct vs spectral >>>> indirect, per-light contribution, etc… >>>> >>>> Cheers, >>>> >>>> Thomas >>>> >>>> On Sat, 4 Dec 2021 at 9:47 PM, Larry Gritz <l...@larrygritz.com >>>> <mailto:l...@larrygritz.com>> wrote: >>>> What's the highest number of color channels you've seen in a single legit >>>> image file? >>>> >>>> If we wanted to have a limit of the number of channels, over which we >>>> would conclude that a file was highly likely to be corrupt or even >>>> malicious... what is a reasonable limit? Is 1024 too low? >>>> >>>> (Assume there is a runtime way to override this limit, in the unlikely >>>> case that a legit file needs more than this number of channels.) >>>> >>>> >>>> -- >>>> Larry Gritz >>>> l...@larrygritz.com <mailto:l...@larrygritz.com> >>>> >>>> >>>> >>>> >>>> _______________________________________________ >>>> Oiio-dev mailing list >>>> Oiio-dev@lists.openimageio.org <mailto:Oiio-dev@lists.openimageio.org> >>>> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org >>>> <http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org> >>>> -- >>>> Thomas Mansencal - colour-science.org <https://www.colour-science.org/> - >>>> thomasmansencal.com >>>> <http://www.thomasmansencal.com/>_______________________________________________ >>>> Oiio-dev mailing list >>>> Oiio-dev@lists.openimageio.org <mailto:Oiio-dev@lists.openimageio.org> >>>> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org >>>> <http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org> >>> >>> -- >>> Larry Gritz >>> l...@larrygritz.com <mailto:l...@larrygritz.com> >>> >>> >>> >>> >>> _______________________________________________ >>> Oiio-dev mailing list >>> Oiio-dev@lists.openimageio.org <mailto:Oiio-dev@lists.openimageio.org> >>> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org >>> <http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org> >>> -- >>> Thomas Mansencal - colour-science.org <https://www.colour-science.org/> - >>> thomasmansencal.com >>> <http://www.thomasmansencal.com/>_______________________________________________ >>> Oiio-dev mailing list >>> Oiio-dev@lists.openimageio.org <mailto:Oiio-dev@lists.openimageio.org> >>> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org >>> <http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org> >> >> -- >> Larry Gritz >> l...@larrygritz.com <mailto:l...@larrygritz.com> >> >> >> >> >> _______________________________________________ >> Oiio-dev mailing list >> Oiio-dev@lists.openimageio.org <mailto:Oiio-dev@lists.openimageio.org> >> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org >> <http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org> > > -- > Larry Gritz > l...@larrygritz.com <mailto:l...@larrygritz.com> > > > > > _______________________________________________ > Oiio-dev mailing list > Oiio-dev@lists.openimageio.org <mailto:Oiio-dev@lists.openimageio.org> > http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org > <http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org> > _______________________________________________ > Oiio-dev mailing list > Oiio-dev@lists.openimageio.org > http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org -- Larry Gritz l...@larrygritz.com
_______________________________________________ Oiio-dev mailing list Oiio-dev@lists.openimageio.org http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
