From: Ming Lu Subject: CA-430810: Newline in repository_proxy_url injects yum/dnf .repo directives Add sanity check in the URLs, proxy username and password configured for YUM/DNF. This is CVE-2026-119780, part of XSA-530. Signed-off-by: Ming Lu Reviewed-by: Andrii Sultanov diff --git a/ocaml/tests/test_repository_helpers.ml b/ocaml/tests/test_repository_helpers.ml index 8ef4ba31445f..ad0f7d856946 100644 --- a/ocaml/tests/test_repository_helpers.ml +++ b/ocaml/tests/test_repository_helpers.ml @@ -250,6 +250,96 @@ module AssertUrlIsValid = Generic.MakeStateless (struct ; (("https://a.b.d.com//", ["c.com"; "d.com"]), Ok ()) ; (("https://a.b.d.com/a/b", ["c.com"; "d.com"]), Ok ()) ; (("https://a.b.d.com/a/b/", ["c.com"; "d.com"]), Ok ()) + ; (* control characters are rejected wherever they appear *) + ( ("http://a.b.c.com/a\n/b", ["c.com"]) + , Error + Api_errors.( + Server_error (invalid_base_url, ["http://a.b.c.com/a\n/b"]) + ) + ) + ; ( ("http://a.b.c.com\r\nX-Injected: 1", ["c.com"]) + , Error + Api_errors.( + Server_error + (invalid_base_url, ["http://a.b.c.com\r\nX-Injected: 1"]) + ) + ) + ; ( ("http://a.b.c\000.com", ["c.com"]) + , Error + Api_errors.( + Server_error (invalid_base_url, ["http://a.b.c\000.com"]) + ) + ) + ; ( ("http://a.b.c.com/\127", ["c.com"]) + , Error + Api_errors.( + Server_error (invalid_base_url, ["http://a.b.c.com/\127"]) + ) + ) + ; ( ("\thttp://a.b.c.com", ["c.com"]) + , Error + Api_errors.(Server_error (invalid_base_url, ["\thttp://a.b.c.com"])) + ) + ] +end) + +module AssertRemotePoolUrlIsValid = Generic.MakeStateless (struct + module Io = struct + type input_t = string + + type output_t = (unit, exn) result + + let string_of_input_t = Fmt.(str "%a" Dump.string) + + let string_of_output_t = + Fmt.(str "%a" Dump.(result ~ok:(any "()") ~error:exn)) + end + + let transform url = + try Ok (assert_remote_pool_url_is_valid ~url) with e -> Error e + + let invalid url = Error Api_errors.(Server_error (invalid_base_url, [url])) + + let tests = + `QuickAndAutoDocumented + [ + ("https://192.168.0.2/repository/enabled", Ok ()) + ; ("https://[::1]/repository/enabled", Ok ()) + ; (* wrong scheme, host or path *) + ( "http://192.168.0.2/repository/enabled" + , invalid "http://192.168.0.2/repository/enabled" + ) + ; ( "https://a.b.c.com/repository/enabled" + , invalid "https://a.b.c.com/repository/enabled" + ) + ; ( "https://192.168.0.2/repository" + , invalid "https://192.168.0.2/repository" + ) + ; (* a newline terminates Uri.path, so the shape check on its own would + accept these and the trailing lines would reach the .repo file *) + ( "https://192.168.0.2/repository/enabled\n\ + [evil]\n\ + baseurl=http://attacker/" + , invalid + "https://192.168.0.2/repository/enabled\n\ + [evil]\n\ + baseurl=http://attacker/" + ) + ; ( "https://192.168.0.2/repository/enabled\nproxy=http://attacker:3128" + , invalid + "https://192.168.0.2/repository/enabled\nproxy=http://attacker:3128" + ) + ; (* other control characters, which the shape check already rejected but + which must keep reporting the same error *) + ( "https://192.168.0.2/repository/enabled\r\nX-Injected: 1" + , invalid "https://192.168.0.2/repository/enabled\r\nX-Injected: 1" + ) + ; ( "https://192.168.0.2\000/repository/enabled" + , invalid "https://192.168.0.2\000/repository/enabled" + ) + ; ( "\thttps://192.168.0.2/repository/enabled" + , invalid "\thttps://192.168.0.2/repository/enabled" + ) ] end) @@ -5576,6 +5666,7 @@ let tests = [ ("update_of_json", UpdateOfJsonTest.tests) ; ("assert_url_is_valid", AssertUrlIsValid.tests) + ; ("assert_remote_pool_url_is_valid", AssertRemotePoolUrlIsValid.tests) ; ("assert_url_is_not_blocked", AssertUrlIsNotBlocked.tests) ; ("write_yum_config", WriteYumConfig.tests) ; ("eval_guidance_for_one_update", EvalGuidanceForOneUpdate.tests) diff --git a/ocaml/xapi/repository.ml b/ocaml/xapi/repository.ml index 6c655de59fa9..cdd060404b08 100644 --- a/ocaml/xapi/repository.ml +++ b/ocaml/xapi/repository.ml @@ -140,11 +140,17 @@ let get_proxy_params ~__context = match (url, username, password_ref) with | url', username', password_ref' when url' <> "" && username' <> "" && password_ref' <> Ref.null -> + let password = Db.Secret.get_value ~__context ~self:password_ref' in + (* The secret value may change after being configured. Re-check it again. *) + if has_control_character password then ( + error + "The password of the repository proxy contains control character(s)" ; + raise Api_errors.(Server_error (invalid_repository_proxy_credential, [])) + ) ; [ Printf.sprintf "proxy=%s" url' ; Printf.sprintf "proxy_username=%s" username' - ; Printf.sprintf "proxy_password=%s" - (Db.Secret.get_value ~__context ~self:password_ref') + ; Printf.sprintf "proxy_password=%s" password ] | url', "", _ when url' <> "" -> [Printf.sprintf "proxy=%s" url'] diff --git a/ocaml/xapi/repository_helpers.ml b/ocaml/xapi/repository_helpers.ml index 13d0eb94770f..5d6bc65558c8 100644 --- a/ocaml/xapi/repository_helpers.ml +++ b/ocaml/xapi/repository_helpers.ml @@ -164,7 +164,21 @@ module DomainNameIncludeIP = struct ) end +let has_control_character s = + let is_control_character c = + let code = Char.code c in + code < 0x20 || code = 0x7f + in + String.exists is_control_character s + +let assert_no_control_chars ~url ~url_error = + if has_control_character url then ( + error "Invalid url: it contains control character(s)" ; + raise Api_errors.(Server_error (url_error, [url])) + ) + let assert_url_is_valid ~url = + assert_no_control_chars ~url ~url_error:Api_errors.invalid_base_url ; try let uri = Uri.of_string url in match Uri.(scheme uri, host uri) with @@ -265,6 +279,7 @@ let get_remote_pool_coordinator_ip url = raise Api_errors.(Server_error (invalid_base_url, [url])) let assert_remote_pool_url_is_valid ~url = + assert_no_control_chars ~url ~url_error:Api_errors.invalid_base_url ; ignore (get_remote_pool_coordinator_ip url : string) let with_pool_repositories f = diff --git a/ocaml/xapi/xapi_pool.ml b/ocaml/xapi/xapi_pool.ml index 63f208b98c41..de1959246996 100644 --- a/ocaml/xapi/xapi_pool.ml +++ b/ocaml/xapi/xapi_pool.ml @@ -4317,13 +4317,18 @@ let configure_repository_proxy ~__context ~self ~url ~username ~password = error "missing password of the repository proxy with a username specified" ; raise Api_errors.(Server_error (invalid_repository_proxy_credential, [])) | u, p when u <> "" && p <> "" -> - if String.contains u '\n' || String.contains p '\n' then ( + if + Repository_helpers.has_control_character u + || Repository_helpers.has_control_character p + then ( error "getting invalid username/password of the repository proxy" ; raise Api_errors.(Server_error (invalid_repository_proxy_credential, [])) ) | _ -> () ) ; + Repository_helpers.assert_no_control_chars ~url + ~url_error:Api_errors.invalid_repository_proxy_url ; let old_secret_ref = Db.Pool.get_repository_proxy_password ~__context ~self in let secret_ref = if password = "" then