Fix error when service is already running and update exception documentation in lib/msf/core/post/windows/services.rb

[kalidor]

Fixes #16603

metasploit-framework/lib/msf/core/post/windows/services.rb

Lines 517 to 564 in 20198a2

	def service_restart(name, start_type=START_TYPE_AUTO, server=nil)
	tried = false
	begin
	status = service_start(name, server)
	if status == Error::SUCCESS
	vprint_good("[#{name}] Service started")
	return true
	else
	raise status
	end
	rescue RuntimeError => s
	if tried
	vprint_error("[#{name}] Unhandled error: #{s}")
	return false
	else
	tried = true
	end
	case s.message.to_i
	when Error::ACCESS_DENIED
	vprint_error("[#{name}] Access denied")
	when Error::INVALID_HANDLE
	vprint_error("[#{name}] Invalid handle")
	when Error::PATH_NOT_FOUND
	vprint_error("[#{name}] Service binary could not be found")
	when Error::SERVICE_ALREADY_RUNNING
	vprint_status("[#{name}] Service already running attempting to stop and restart")
	stopped = service_stop(name, server)
	if ((stopped == Error::SUCCESS) || (stopped == Error::SERVICE_NOT_ACTIVE))
	retry
	else
	vprint_error("[#{name}] Service disabled, unable to change start type Error: #{stopped}")
	end
	when Error::SERVICE_DISABLED
	vprint_status("[#{name}] Service disabled attempting to set to manual")
	if (service_change_config(name, {:starttype => start_type}, server) == Error::SUCCESS)
	retry
	else
	vprint_error("[#{name}] Service disabled, unable to change start type")
	end
	else
	vprint_error("[#{name}] Unhandled error: #{s}")
	return false
	end
	end
	end

metasploit-framework/lib/msf/core/post/windows/services.rb

Lines 423 to 431 in 20198a2

	def service_start(name, server=nil)
	open_sc_manager(:host=>server, :access=>"SC_MANAGER_CONNECT") do |manager|
	open_service_handle(manager, name, "SERVICE_START") do |service_handle|
	retval = advapi32.StartServiceA(service_handle,0,nil)
	return retval["GetLastError"]
	end
	end
	end

service_start() function should return Error::SERVICE_ALREADY_RUNNING as string or raise Error::SERVICE_ALREADY_RUNNING, to get catch by rescue RuntimeError => s block and don't raise an error.

I suppose raising an error is better, since the function will keep returning only Integer.

[gwillcox-r7]

This will need a secondary review, but @kalidor if you could confirm these changes work for you, I don't think your current steps provide enough info to fully replicate your issue so I'd appreciate a demo video or similar showcasing that these fixes work for you before I land this.

@bwatters-r7 Any chance you could take a look over this before I land it? Apologies in advance for asking you to take a second look at my PRs this week 😅

[kalidor]

Hi, i can confirm those changes work fine. Here are two videos: https://unixed.fr/~gch/msf_service.html. The first one used the actual version of services.rb, the second one uses the updates version of services.rb.

[gwillcox-r7]

Alright thanks @kalidor! Videos look good, will land this now. Should be available in this week's release.

[gwillcox-r7]

Release Notes

Previously if a user restarted a service using lib/msf/core/post/windows/services.rb an exception would be thrown as a integer instead of as a string, which would cause an error to occur. This has been fixed by rewriting the code for the service_restart to use more appropriate logic. Additionally the documentation has been updated for lib/msf/core/post/windows/services.rb to note which functions may throw exceptions.