Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Maximum filename length is wrong #247

Closed
guijan opened this issue Mar 19, 2023 · 0 comments
Closed

Maximum filename length is wrong #247

guijan opened this issue Mar 19, 2023 · 0 comments

Comments

@guijan
Copy link
Contributor

guijan commented Mar 19, 2023

Scrot uses this macro to check if input/output filenames are within a certain size:

scrot/src/options.c

Lines 61 to 66 in 70a7540

#define STR_LEN_MAX_FILENAME(msg, fileName) do { \
if (strlen((fileName)) > MAX_FILENAME) { \
errx(EXIT_FAILURE, #msg " filename too long, must be " \
"less than %d characters", MAX_FILENAME); \
} \
} while(0)

MAX_FILENAME = 256, // characters

The issue is that a maximum length of 256 is hardcoded, but the maximum filename length is a very complex issue. 256 could be too much or too little depending on the operating system, the file system, and even what directory the file is created in. The solution is that the code handling input/output filenames should pass around an alloc storage duration buffer and dynamically grow it as needed throughout the program's execution. Then, when the file is finally created, the system will return error if it fails to create the file.

Also, that code didn't have to be a macro.

#229 partly fixes this bug along with a host of other bugs.

There is much more code around handling filenames which needs to be read carefully to check if it's free from this bug or not. The code that handles the output file and its filename is extremely buggy as many issues attest: #241 #244 #246 #228 #226 #223

N-R-K added a commit to N-R-K/scrot that referenced this issue May 6, 2023
256 (or NAME_MAX) is not a reliable constant. furthermore, the
"fileName" in this case can be a full path as well, so checking against
NAME_MAX would be wrong regardless.

Closes: resurrecting-open-source-projects#247
@N-R-K N-R-K closed this as completed in d91991f May 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant