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.

Sign up for GitHub

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

Jump to bottom

Authfile Leaking File Descriptor #136

Closed

marc-sensenich opened this issue Apr 2, 2018 · 5 comments

Contributor

marc-sensenich commented Apr 2, 2018

When the authfile is successfully opened, the file descriptor is not closed and leaks file descriptors. The file descriptor should be closed after this line; https://github.com/Yubico/yubico-pam/blob/master/util.c#L177

$ lsof /path/to/authfile | wc -l
8
# Login to the system using yubico-pam a few more times
$ lsof /path/to/authfile | wc -l
25

Member

klali commented Apr 3, 2018

Thanks!

klali closed this as completed in

0f6ceab

Contributor Author

marc-sensenich commented Apr 3, 2018

Thanks @klali! What would be the expected date of the next release for this to be included?

Member

klali commented Apr 4, 2018

Unknown at this time, we just made a release and given the overhead I'd like to collect a few more fixes before spending the time to make another.

Contributor

kbabioch commented Apr 4, 2018

This was introduced with d9780ea, which first appeared in 2.18. Beforehand the file handle was correctly closed in pam_yubico.c

Contributor

kbabioch commented Apr 5, 2018

CVE-2018-9275 has been assigned to this.

kbabioch added a commit to kbabioch/yubico-pam that referenced this issue


          Open file descriptors with O_CLOEXEC

7f33526

This opens any file descriptors with the O_CLOEXEC flag, which will make sure
that file descriptors won't be leaked into any child process. This was
previously an issue due to a forgotten fclose() (Yubico#136).

kbabioch added a commit to kbabioch/yubico-pam that referenced this issue


          Use mkotemp() instead of mkstemp()

35f8be0

This uses mkotemp() instead of mkstemp(), passing along the `O_CLOEXEC` flag,
which makes sure that the file descriptor is closed and won't be leaked into
any child process, which was previously an issue due to a missing fclose()
(Yubico#136).

kbabioch added a commit to kbabioch/yubico-pam that referenced this issue


          Add "e" flag to fopen() calls

2c7a00e

This adds the `e` flag to fopen() calls, making sure the `O_CLOEXEC` flag is
used. This makes sure that the file descriptor is being closed and not leaked
into child processes. This was an issues previously due to a missing fclose()
(Yubico#136).

kbabioch added a commit to kbabioch/yubico-pam that referenced this issue


          Use mkotemp() instead of mkstemp()

19b6b36

This uses mkostemp() instead of mkstemp(), passing along the `O_CLOEXEC` flag,
which makes sure that the file descriptor is closed and won't be leaked into
any child process, which was previously an issue due to a missing fclose()
(Yubico#136).

kbabioch added a commit to kbabioch/yubico-pam that referenced this issue


          Add "e" flag to fopen() calls

2c5d4d4

This adds the `e` flag to fopen() calls, making sure the `O_CLOEXEC` flag is
used. This makes sure that the file descriptor is being closed and not leaked
into child processes. This was an issues previously due to a missing fclose()
(Yubico#136).

kbabioch added a commit to kbabioch/yubico-pam that referenced this issue


          Open file descriptors with O_CLOEXEC

dcb4e4c

This opens any file descriptors with the O_CLOEXEC flag, which will make sure
that file descriptors won't be leaked into any child process. This was
previously an issue due to a forgotten fclose() (Yubico#136).

kbabioch added a commit to kbabioch/yubico-pam that referenced this issue


          Use mkotemp() instead of mkstemp()

adc6418

This uses mkostemp() instead of mkstemp(), passing along the `O_CLOEXEC` flag,
which makes sure that the file descriptor is closed and won't be leaked into
any child process, which was previously an issue due to a missing fclose()
(Yubico#136).

kbabioch added a commit to kbabioch/yubico-pam that referenced this issue


          Add "e" flag to fopen() calls

4ba40de

This adds the `e` flag to fopen() calls, making sure the `O_CLOEXEC` flag is
used. This makes sure that the file descriptor is being closed and not leaked
into child processes. This was an issues previously due to a missing fclose()
(Yubico#136).

kbabioch mentioned this issue

Make use of the O_CLOEXEC flag when dealing with file descriptors #145

Merged

kbabioch added a commit to kbabioch/yubico-pam that referenced this issue


          Open file descriptors with O_CLOEXEC

22f7129

This opens any file descriptors with the O_CLOEXEC flag, which will make sure
that file descriptors won't be leaked into any child process. This was
previously an issue due to a forgotten fclose() (Yubico#136).

kbabioch added a commit to kbabioch/yubico-pam that referenced this issue


          Use mkotemp() instead of mkstemp()

5b2c9c3

This uses mkostemp() instead of mkstemp(), passing along the `O_CLOEXEC` flag,
which makes sure that the file descriptor is closed and won't be leaked into
any child process, which was previously an issue due to a missing fclose()
(Yubico#136).

kbabioch added a commit to kbabioch/yubico-pam that referenced this issue


          Add "e" flag to fopen() calls

95b107d

This adds the `e` flag to fopen() calls, making sure the `O_CLOEXEC` flag is
used. This makes sure that the file descriptor is being closed and not leaked
into child processes. This was an issues previously due to a missing fclose()
(Yubico#136).

kbabioch added a commit to kbabioch/yubico-pam that referenced this issue


          Add "e" flag to fopen() calls

a3e349a

This adds the `e` flag to fopen() calls, making sure the `O_CLOEXEC` flag is
used. This makes sure that the file descriptor is being closed and not leaked
into child processes. This was an issues previously due to a missing fclose()
(Yubico#136).

kbabioch added a commit to kbabioch/yubico-pam that referenced this issue


          Use mkotemp() instead of mkstemp()

0a75bcd

This uses mkostemp() instead of mkstemp(), passing along the `O_CLOEXEC` flag,
which makes sure that the file descriptor is closed and won't be leaked into
any child process, which was previously an issue due to a missing fclose()
(Yubico#136).

kbabioch added a commit to kbabioch/yubico-pam that referenced this issue


          Open file descriptors with O_CLOEXEC

418dfef

This opens any file descriptors with the O_CLOEXEC flag, which will make sure
that file descriptors won't be leaked into any child process. This was
previously an issue due to a forgotten fclose() (Yubico#136).

kbabioch added a commit to kbabioch/yubico-pam that referenced this issue


          Open file descriptors with O_CLOEXEC

e170da5

This opens any file descriptors with the O_CLOEXEC flag, which will make sure
that file descriptors won't be leaked into any child process. This was
previously an issue due to a forgotten fclose() (Yubico#136).

kbabioch added a commit to kbabioch/yubico-pam that referenced this issue


          Add "e" flag to fopen() calls

c727fc4

This adds the `e` flag to fopen() calls, making sure the `O_CLOEXEC` flag is
used. This makes sure that the file descriptor is being closed and not leaked
into child processes. This was an issues previously due to a missing fclose()
(Yubico#136).

kbabioch added a commit to kbabioch/yubico-pam that referenced this issue


          Use mkotemp() instead of mkstemp()

e0b4cf6

This uses mkostemp() instead of mkstemp(), passing along the `O_CLOEXEC` flag,
which makes sure that the file descriptor is closed and won't be leaked into
any child process, which was previously an issue due to a missing fclose()
(Yubico#136).

kbabioch added a commit to kbabioch/yubico-pam that referenced this issue


          Open file descriptors with O_CLOEXEC

079b975

This opens any file descriptors with the O_CLOEXEC flag, which will make sure
that file descriptors won't be leaked into any child process. This was
previously an issue due to a forgotten fclose() (Yubico#136).

kbabioch added a commit to kbabioch/yubico-pam that referenced this issue


          Add "e" flag to fopen() calls

d51124e

This adds the `e` flag to fopen() calls, making sure the `O_CLOEXEC` flag is
used. This makes sure that the file descriptor is being closed and not leaked
into child processes. This was an issues previously due to a missing fclose()
(Yubico#136).

kbabioch added a commit to kbabioch/yubico-pam that referenced this issue


          Use mkotemp() instead of mkstemp()

0b595ee

This uses mkostemp() instead of mkstemp(), passing along the `O_CLOEXEC` flag,
which makes sure that the file descriptor is closed and won't be leaked into
any child process, which was previously an issue due to a missing fclose()
(Yubico#136).

kbabioch added a commit to kbabioch/yubico-pam that referenced this issue


          Open file handlers manually using open() and fdopen() instead of usin…

4d45bd3

…g fopen()

A previous commit (d51124e) added the `e` flag to the `fopen()` calls. However
this flag is not supported on all platforms (MacOS) and will be silently
dropped (see Yubico#145). This patch works around those issues by manually opening
the file descriptor using `open()` with the `O_CLOEXEC` flag, and invoking
`fd_open()` on the resulting file descriptor to open an appropriate `FILE`
stream.

This makes sure that all files used by pam_yubico will be opened with the
`O_CLOEXEC` flag on all supported platforms to mitigate issues with missing
`fclose()` invocation (see Yubico#136).

kbabioch added a commit to kbabioch/yubico-pam that referenced this issue


          Open file handlers manually using open() and fdopen() instead of usin…

e5bd2ef

…g fopen()

A previous commit (d51124e) added the `e` flag to the `fopen()` calls. However
this flag is not supported on all platforms (MacOS) and will be silently
dropped (see Yubico#145). This patch works around those issues by manually opening
the file descriptor using `open()` with the `O_CLOEXEC` flag, and invoking
`fd_open()` on the resulting file descriptor to open an appropriate `FILE`
stream.

This makes sure that all files used by pam_yubico will be opened with the
`O_CLOEXEC` flag on all supported platforms to mitigate issues with missing
`fclose()` invocation (see Yubico#136).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment