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.

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

Hardening #54

Merged
merged 3 commits into from
Dec 5, 2018
Merged

Hardening #54

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f31b12a
use absolute path and prevent shell injection
jreidinger Dec 5, 2018
e5ceded
changes
jreidinger Dec 5, 2018
fe5e530
fix typo and make rubocop happy
jreidinger Dec 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions package/yast2-ftp-server.changes
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
-------------------------------------------------------------------
Wed Dec 5 11:43:58 UTC 2018 - jreidinger@suse.com

- always use absolute path to binaries (bsc#1118291)
- escape properly shell arguments (bsc#1118291)
- 4.1.6

-------------------------------------------------------------------
Sun Nov 25 03:00:38 UTC 2018 - Stasiek Michalski <hellcp@mailbox.org>

Expand Down
2 changes: 1 addition & 1 deletion package/yast2-ftp-server.spec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@


Name: yast2-ftp-server
Version: 4.1.5
Version: 4.1.6
Release: 0

BuildRoot: %{_tmppath}/%{name}-%{version}-build
Expand Down
74 changes: 15 additions & 59 deletions src/modules/FtpServer.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
require "yast"
require "yast2/system_service"
require "y2firewall/firewalld"
require "shellwords"
require "fileutils"

module Yast
# Configure vsftpd: https://security.appspot.com/vsftpd.html
Expand Down Expand Up @@ -193,7 +195,7 @@ def ReadVSFTPDUpload
result = false
command = ""
if @anon_homedir != ""
command = Ops.add(Ops.add("ls -l ", @anon_homedir), " | grep upload")
command = "/usr/bin/ls -l #{@anon_homedir.shellescape} | /usr/bin/grep upload")
end
if command != ""
options = Convert.to_map(
Expand Down Expand Up @@ -273,10 +275,7 @@ def ReadPermisionUplaod
end

if @anon_homedir != "" && @pure_ftp_allowed_permissios_upload != -1
command = Ops.add(
Ops.add(Ops.add("ls -l ", directory), " | grep "),
upload_dir
)
command = "/usr/bin/ls -l #{directory.shellescape} | /usr/bin/grep #{upload_dir.shellescape}"
end
if command != ""
options = Convert.to_map(
Expand Down Expand Up @@ -494,7 +493,6 @@ def WriteSettings
# It is necessary if user want to allow uploading for anonymous
# @return [Boolean] result of function (true/false)
def WriteUpload
result = true
command = ""
upload = ""
authentication = Builtins.tointeger(Ops.get(@EDIT_SETTINGS, "AnonAuthen"))
Expand All @@ -512,73 +510,31 @@ def WriteUpload
"/upload"
end
end
command = "dir=`ls "
command = Ops.add(command, @anon_homedir)
command = Ops.add(
command,
" | grep upload`; if [ -z $dir ]; then mkdir "
)
command = Ops.add(
Ops.add(Ops.add(command, @anon_homedir), upload),
"; chown "
)

if Ops.get(@EDIT_SETTINGS, "GuestUser") != ""
command = Ops.add(
Ops.add(Ops.add(command, Ops.get(@EDIT_SETTINGS, "GuestUser")), ":"),
Ops.get(@EDIT_SETTINGS, "GuestUser")
)
user = @EDIT_SETTINGS["GuestUser"]
elsif Ops.get(@EDIT_SETTINGS, "FTPUser") != ""
command = Ops.add(
Ops.add(Ops.add(command, Ops.get(@EDIT_SETTINGS, "FTPUser")), ":"),
Ops.get(@EDIT_SETTINGS, "FTPUser")
)
user = @EDIT_SETTINGS["FTPUser"]
end

command = Ops.add(
Ops.add(Ops.add(Ops.add(command, " "), @anon_homedir), upload),
"; chmod 766 "
)
command = Ops.add(
Ops.add(
Ops.add(
Ops.add(
Ops.add(Ops.add(command, @anon_homedir), upload),
"; else chmod 766 "
),
@anon_homedir
),
upload
),
"; fi"
)
# "dir=`ls /srv/ftp/ | grep upload`; if [ -z $dir ]; then echo $dir; mkdir /srv/ftp/upload;
# chown ftp:ftp /srv/ftp/upload/; chmod 755 /srv/ftp/upload; else chmod 766 /srv/ftp/upload/; fi"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is old script encoded in Ops.add.

Builtins.y2milestone(
"[ftp-server] (WriteUpload) bash command for creating upload dir : %1",
command
)
options = Convert.to_map(
SCR.Execute(path(".target.bash_output"), command)
)
result = if Ops.get(options, "exit").zero?
true
else
false
dir = @anon_homedir + upload

if !File.exist?(dir)
FileUtils.mkdir(dir)
FileUtils.chown(user, user, dir) if user
end
else
result = true

FileUtils.chmod(0766, dir)
end
# restart/reaload daemons...
Service.restart("vsftpd") if Service.active?("vsftpd")

# update permissions for home directory if upload is enabled...
if @pure_ftp_allowed_permissios_upload != -1 && @change_permissions
command = Ops.add("chmod 755 ", @anon_homedir)
SCR.Execute(path(".target.bash_output"), command)
FileUtils.chmod(0755, @anon_homedir)
end

result
true
end

# read value from PURE_EDIT_SETTINGS
Expand Down