* [PATCH] semodule: avoid toctou on output module
@ 2022-05-20 13:19 Christian Göttsche
2022-05-29 22:52 ` Nicolas Iooss
0 siblings, 1 reply; 3+ messages in thread
From: Christian Göttsche @ 2022-05-20 13:19 UTC (permalink / raw)
To: selinux
Do not check for file existence and open afterwards, open with the
exclusive flag (supported in Glibc and musl 0.9.6 and also standardized
in C11).
Found by GitHub CodeQL.
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
policycoreutils/semodule/semodule.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/policycoreutils/semodule/semodule.c b/policycoreutils/semodule/semodule.c
index 1ed8e690..48bc28dd 100644
--- a/policycoreutils/semodule/semodule.c
+++ b/policycoreutils/semodule/semodule.c
@@ -550,15 +550,12 @@ int main(int argc, char *argv[])
goto cleanup_extract;
}
- if (access(output_path, F_OK) == 0) {
- fprintf(stderr, "%s: %s is already extracted with extension %s.\n", argv[0], mode_arg, lang_ext);
- result = -1;
- goto cleanup_extract;
- }
-
- output_fd = fopen(output_path, "w");
+ output_fd = fopen(output_path, "wx");
if (output_fd == NULL) {
- fprintf(stderr, "%s: Unable to open %s\n", argv[0], output_path);
+ if (errno == EEXIST)
+ fprintf(stderr, "%s: %s is already extracted with extension %s.\n", argv[0], mode_arg, lang_ext);
+ else
+ fprintf(stderr, "%s: Unable to open %s: %s\n", argv[0], output_path, strerror(errno));
result = -1;
goto cleanup_extract;
}
--
2.36.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] semodule: avoid toctou on output module
2022-05-20 13:19 [PATCH] semodule: avoid toctou on output module Christian Göttsche
@ 2022-05-29 22:52 ` Nicolas Iooss
2022-06-02 13:04 ` James Carter
0 siblings, 1 reply; 3+ messages in thread
From: Nicolas Iooss @ 2022-05-29 22:52 UTC (permalink / raw)
To: Christian Göttsche; +Cc: SElinux list
On Fri, May 20, 2022 at 3:20 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Do not check for file existence and open afterwards, open with the
> exclusive flag (supported in Glibc and musl 0.9.6 and also standardized
> in C11).
>
> Found by GitHub CodeQL.
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
This looks good to me.
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Thanks!
> ---
> policycoreutils/semodule/semodule.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/policycoreutils/semodule/semodule.c b/policycoreutils/semodule/semodule.c
> index 1ed8e690..48bc28dd 100644
> --- a/policycoreutils/semodule/semodule.c
> +++ b/policycoreutils/semodule/semodule.c
> @@ -550,15 +550,12 @@ int main(int argc, char *argv[])
> goto cleanup_extract;
> }
>
> - if (access(output_path, F_OK) == 0) {
> - fprintf(stderr, "%s: %s is already extracted with extension %s.\n", argv[0], mode_arg, lang_ext);
> - result = -1;
> - goto cleanup_extract;
> - }
> -
> - output_fd = fopen(output_path, "w");
> + output_fd = fopen(output_path, "wx");
> if (output_fd == NULL) {
> - fprintf(stderr, "%s: Unable to open %s\n", argv[0], output_path);
> + if (errno == EEXIST)
> + fprintf(stderr, "%s: %s is already extracted with extension %s.\n", argv[0], mode_arg, lang_ext);
> + else
> + fprintf(stderr, "%s: Unable to open %s: %s\n", argv[0], output_path, strerror(errno));
> result = -1;
> goto cleanup_extract;
> }
> --
> 2.36.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] semodule: avoid toctou on output module
2022-05-29 22:52 ` Nicolas Iooss
@ 2022-06-02 13:04 ` James Carter
0 siblings, 0 replies; 3+ messages in thread
From: James Carter @ 2022-06-02 13:04 UTC (permalink / raw)
To: Nicolas Iooss; +Cc: Christian Göttsche, SElinux list
On Mon, May 30, 2022 at 1:00 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>
> On Fri, May 20, 2022 at 3:20 PM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> >
> > Do not check for file existence and open afterwards, open with the
> > exclusive flag (supported in Glibc and musl 0.9.6 and also standardized
> > in C11).
> >
> > Found by GitHub CodeQL.
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
>
> This looks good to me.
>
> Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
>
Merged.
Thanks,
Jim
> Thanks!
>
> > ---
> > policycoreutils/semodule/semodule.c | 13 +++++--------
> > 1 file changed, 5 insertions(+), 8 deletions(-)
> >
> > diff --git a/policycoreutils/semodule/semodule.c b/policycoreutils/semodule/semodule.c
> > index 1ed8e690..48bc28dd 100644
> > --- a/policycoreutils/semodule/semodule.c
> > +++ b/policycoreutils/semodule/semodule.c
> > @@ -550,15 +550,12 @@ int main(int argc, char *argv[])
> > goto cleanup_extract;
> > }
> >
> > - if (access(output_path, F_OK) == 0) {
> > - fprintf(stderr, "%s: %s is already extracted with extension %s.\n", argv[0], mode_arg, lang_ext);
> > - result = -1;
> > - goto cleanup_extract;
> > - }
> > -
> > - output_fd = fopen(output_path, "w");
> > + output_fd = fopen(output_path, "wx");
> > if (output_fd == NULL) {
> > - fprintf(stderr, "%s: Unable to open %s\n", argv[0], output_path);
> > + if (errno == EEXIST)
> > + fprintf(stderr, "%s: %s is already extracted with extension %s.\n", argv[0], mode_arg, lang_ext);
> > + else
> > + fprintf(stderr, "%s: Unable to open %s: %s\n", argv[0], output_path, strerror(errno));
> > result = -1;
> > goto cleanup_extract;
> > }
> > --
> > 2.36.1
> >
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-06-02 13:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-20 13:19 [PATCH] semodule: avoid toctou on output module Christian Göttsche
2022-05-29 22:52 ` Nicolas Iooss
2022-06-02 13:04 ` James Carter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.