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 <ke...@8t8.us>
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 <vinc...@vinc17.net> - 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)
