Skip to content
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.

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

[3.5.x] Implemented safe_open() in order to avoid symlink race conditions. #1116

Merged
merged 1 commit into from
Nov 26, 2013

Conversation

kacf
Copy link
Contributor

@kacf kacf commented Nov 26, 2013

It makes sure that we do not follow unsafe symlinks, that is a link
that belongs to a user, but has a target owned by a different user.
It does this in a way that avoids race conditions (between testing if
a file is a link, and opening that file, there is a small window of
opportunity).

Also implements safe_chdir(), to also safeguard chdir() operations.

Fixes issue #3693.
(cherry picked from commit 995b9d1)

Conflicts:

cf-agent/files_editline.c
cf-agent/verify_files.c
cf-agent/verify_packages.c
cf-execd/cf-execd-runner.c
cf-key/cf-key-functions.c
cf-serverd/server_common.c
configure.ac
libcfnet/client_code.c
libpromises/cf3lex.l
libpromises/env_context.c
libpromises/parser.c
libpromises/pipes_unix.c
libpromises/sysinfo.c
libpromises/verify_reports.c
libutils/hashes.c
tests/unit/Makefile.am

It makes sure that we do not follow unsafe symlinks, that is a link
that belongs to a user, but has a target owned by a different user.
It does this in a way that avoids race conditions (between testing if
a file is a link, and opening that file, there is a small window of
opportunity).

Also implements safe_chdir(), to also safeguard chdir() operations.

Fixes issue cfengine#3693.
(cherry picked from commit 995b9d1)

Conflicts:

	cf-agent/files_editline.c
	cf-agent/verify_files.c
	cf-agent/verify_packages.c
	cf-execd/cf-execd-runner.c
	cf-key/cf-key-functions.c
	cf-serverd/server_common.c
	configure.ac
	libcfnet/client_code.c
	libpromises/cf3lex.l
	libpromises/env_context.c
	libpromises/parser.c
	libpromises/pipes_unix.c
	libpromises/sysinfo.c
	libpromises/verify_reports.c
	libutils/hashes.c
	tests/unit/Makefile.am
kacf added a commit that referenced this pull request Nov 26, 2013
[3.5.x] Implemented safe_open() in order to avoid symlink race conditions.
@kacf kacf merged commit 988ed70 into cfengine:3.5.x Nov 26, 2013
@kacf kacf deleted the safe_open_3.5.x branch November 26, 2013 07:59
@jooooooon
Copy link
Contributor

Hi, the bug this fixes gives me a 403 error. Any chance of knowing what actual bug(s) this addresses? I don't necessarily need access to your internal tickets (although this is an open source project, it would make sense), but just a summary of what this fixes would be nice... Thanks... :)

@kacf
Copy link
Contributor Author

kacf commented Mar 5, 2014

All the information you need should be there in the commit message. We don't want to follow symlinks owned by one user, pointing to resources owned by a different user. For example, imagine you are using CFEngine to edit /home/joe/.ssh/authorized_keys, but joe has made .ssh a symlink to /root/.ssh. CFEngine runs with root privileges, so that would edit root's authorized_keys instead, which could get nasty. So it's an extra safeguard for those who might have such policies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants