On 2024-07-03 12:37:27 +0000, Matthias Klose wrote: > gcc -DPKGDATADIR=\"/usr/share/mutt\" -DSYSCONFDIR=\"/etc\" > -DBINDIR=\"/usr/bin\" -DMUTTLOCALEDIR=\"/usr/share/locale\" -DHAVE_CONFIG_H=1 > -I. -I../.. -I. -I../.. -I../../imap -Wdate-time -D_FORTIFY_SOURCE=2 > -isystem /usr/include/mit-krb5 -Wall -pedantic -Wno-long-long -g -O2 > -Werror=implicit-function-declaration -ffile-prefix-map=/<<PKGBUILDDIR>>=. > -fstack-protector-strong -fstack-clash-protection -Wformat > -Werror=format-security -fcf-protection -c -o buffy.o ../../buffy.c > ../../attach.c: In function ‘mutt_view_attachment’: > ../../attach.c:392:14: error: passing argument 1 of ‘chmod’ from incompatible > pointer type [-Wincompatible-pointer-types] > 392 | chmod (tempfile, 0400); > | ^~~~~~~~ > | | > | BUFFER * > In file included from ../../mutt.h:30, > from ../../attach.c:24: > /usr/include/x86_64-linux-gnu/sys/stat.h:352:31: note: expected ‘const char > *’ but argument is of type ‘BUFFER *’ > 352 | extern int chmod (const char *__file, __mode_t __mode) > | ~~~~~~~~~~~~^~~~~~ > ../../attach.c:604:11: error: passing argument 1 of ‘chmod’ from incompatible > pointer type [-Wincompatible-pointer-types] > 604 | chmod(tempfile, 0600); > | ^~~~~~~~ > | | > | BUFFER * > /usr/include/x86_64-linux-gnu/sys/stat.h:352:31: note: expected ‘const char > *’ but argument is of type ‘BUFFER *’ > 352 | extern int chmod (const char *__file, __mode_t __mode) > | ~~~~~~~~~~~~^~~~~~
This code comes from the following patch: upstream/528233-readonly-open.patch which dates back to 2014. But in 2018, the type of tempfile changed: commit c619d5cbc428de3632c55bac2e79c7b06d6a030a Author: Kevin McCarthy <[email protected]> Date: 2018-10-14 21:52:30 +0200 Convert mutt_get_tmp_attachment to use BUFFER. diff --git a/attach.c b/attach.c index 2a2661f4..89574f50 100644 --- a/attach.c +++ b/attach.c @@ -46,38 +46,46 @@ int mutt_get_tmp_attachment (BODY *a) { char type[STRING]; - char tempfile[_POSIX_PATH_MAX]; - rfc1524_entry *entry = rfc1524_new_entry(); + BUFFER *tempfile = NULL; + rfc1524_entry *entry = NULL; [...] and the patch became incorrect as a consequence (BUFFER is a struct). If this was still working, this was only by chance. Since the meaning of a pointer changed, this could give memory corruption and things like that. Here, it seems that in the patch, "tempfile" should be replaced by "mutt_b2s (tempfile)", because the current code also has safe_fopen(mutt_b2s (tempfile), "w") and mutt_rename_file (mutt_b2s (tempfile), a->filename) It happens that with the chosen structure, this does not change the address: /* Convert a buffer to a const char * "string" */ #define mutt_b2s(b) (b->data ? (const char *)b->data : "") and data is the first field of the structure: typedef struct { char *data; /* pointer to data */ char *dptr; /* current read/write position */ size_t dsize; /* length of data */ } BUFFER; But this could have been different. I'm wondering why the code still compiled at that time. -- Vincent Lefèvre <[email protected]> - Web: <https://www.vinc17.net/> 100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/> Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)
