| Message ID | 20220520131952.12286-1-cgzones@googlemail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 6d02b2fa2995 |
| Headers | show |
| Series | semodule: avoid toctou on output module | expand |
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 >
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 > > >
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; }
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(-)