search

resurrecting-open-source-projects / scrot

SCReenshOT - command line screen capture utility
Other
511 stars 51 forks source link

`access()` TOCTTOU #244

Open guijan opened 1 year ago

guijan commented 1 year ago

The access() function suffers from a time of check to time of use issue. Nothing guarantees us that what access() could access will be there when we try to open(), or that what it couldn't access won't be there. The code shouldn't ask if it can do something, it should just do it and check the return value. This function shouldn't exist and its callers should be reviewed: https://github.com/resurrecting-open-source-projects/scrot/blob/70a7540cfdc1000db8d400883c2eece386b5bf91/src/options.c#L332-L336 access() is also used elsewhere, use grep to find the uses: https://github.com/resurrecting-open-source-projects/scrot/blob/70a7540cfdc1000db8d400883c2eece386b5bf91/src/scrot.c#L486 https://github.com/resurrecting-open-source-projects/scrot/blob/70a7540cfdc1000db8d400883c2eece386b5bf91/src/scrot.c#L455

N-R-K commented 6 months ago

Imlib2 v1.11.0 was released in 2023-03-09. I think we can probably start requiring it as a minimum version and start using better APIs like imlib_save_image_fd().

guijan commented 6 months ago

https://repology.org/project/imlib2/versions

Requiring very recent versions of things causes 2 issues to my knowledge. It often makes it difficult to develop, one might need to figure out how to compile imlib2 or some other dependency and how to tell scrot to use the local version instead of the system one before being able to work on scrot, this might involve fixing portability issues in the program that package maintainers would otherwise handle. I'm running into a lot of that on the Dolphin emulator right now, they move fast and break fast. It can also cause issues with programs requiring different versions of other programs that can't coexist, such as a program and its library requiring different versions of another library that can't coexist in the same address space, or different versions of display servers that can't coexist either.

The solution is to keep some degree of backwards compatibility, just enough that being limited by it isn't worse than the problems it solves. I suggest only depending on versions that are both older than 1 year and available on Debian Stable's repositories. imlib2 v1.11 is older than 1 year, but not in Debian Stable yet.

N-R-K commented 6 months ago

I also generally do not like the "move fast break things" attitude some developers have and am aware of how certain distributions can make it a PITA to mess with library versions.

I figured a 1 year old release would be enough, but if we want to wait for it to be available in debian stable as a criteria as well then that's fine by me.

And on that note, it seems like there's no timeline for when "trixie" (which contains imlib2 v1.11.0+) will be released. According to https://release.debian.org

The release date for Debian 13 "trixie" hasn't been decided yet.