From 32001ff3ce268cb39708cc87f1a3a9914bdaffcc Mon Sep 17 00:00:00 2001 From: Oleg Date: Thu, 28 Mar 2019 10:22:57 -0400 Subject: [PATCH 1/4] fixes TOCTOU bug in log --- lib/bap/bap_log.ml | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/lib/bap/bap_log.ml b/lib/bap/bap_log.ml index 158299f6b..265327a86 100644 --- a/lib/bap/bap_log.ml +++ b/lib/bap/bap_log.ml @@ -54,13 +54,26 @@ let print_message ppf msg = | Error -> eprintf "%s@\n%!" msg.message | _ -> () +let lock log_folder = + let lock = log_folder / "lock" in + let lock = Unix.openfile lock Unix.[O_RDWR; O_CREAT] 0o666 in + Unix.lockf lock Unix.F_LOCK 0; + lock + +let unlock lock = + Unix.lockf lock Unix.F_ULOCK 0; + Unix.close lock + let open_log_channel user_dir = try let log_folder = log_folder user_dir in mkdir log_folder; let file = log_folder / "log" in - if Sys.file_exists file - then rotate max_logs file; + let lock = lock log_folder in + protect ~f:(fun () -> + if Sys.file_exists file + then rotate max_logs file) + ~finally:(fun () -> unlock lock); let ch = Out_channel.create file in formatter_of_out_channel ch with exn -> From 81441289f5c8ad784b4e7bd6892459bad3401a47 Mon Sep 17 00:00:00 2001 From: Oleg Date: Thu, 28 Mar 2019 11:18:15 -0400 Subject: [PATCH 2/4] protect mkdir too --- lib/bap/bap_log.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/bap/bap_log.ml b/lib/bap/bap_log.ml index 265327a86..c9f27334e 100644 --- a/lib/bap/bap_log.ml +++ b/lib/bap/bap_log.ml @@ -67,10 +67,10 @@ let unlock lock = let open_log_channel user_dir = try let log_folder = log_folder user_dir in - mkdir log_folder; let file = log_folder / "log" in let lock = lock log_folder in protect ~f:(fun () -> + mkdir log_folder; if Sys.file_exists file then rotate max_logs file) ~finally:(fun () -> unlock lock); From 0f00f996a639c8f46baa16f5448c816e903af9c6 Mon Sep 17 00:00:00 2001 From: Oleg Date: Fri, 29 Mar 2019 09:18:30 -0400 Subject: [PATCH 3/4] moved lock in tmp dir --- lib/bap/bap_log.ml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/bap/bap_log.ml b/lib/bap/bap_log.ml index c9f27334e..cd42fe88d 100644 --- a/lib/bap/bap_log.ml +++ b/lib/bap/bap_log.ml @@ -54,9 +54,9 @@ let print_message ppf msg = | Error -> eprintf "%s@\n%!" msg.message | _ -> () -let lock log_folder = - let lock = log_folder / "lock" in - let lock = Unix.openfile lock Unix.[O_RDWR; O_CREAT] 0o666 in +let lock () = + let file = Filename.get_temp_dir_name () / "bap_lock" in + let lock = Unix.openfile file Unix.[O_RDWR; O_CREAT] 0o666 in Unix.lockf lock Unix.F_LOCK 0; lock @@ -68,7 +68,7 @@ let open_log_channel user_dir = try let log_folder = log_folder user_dir in let file = log_folder / "log" in - let lock = lock log_folder in + let lock = lock () in protect ~f:(fun () -> mkdir log_folder; if Sys.file_exists file From 4747f8c3b55eacbebc9c5715930c58a4dd26dc0d Mon Sep 17 00:00:00 2001 From: Oleg Date: Fri, 29 Mar 2019 12:43:03 -0400 Subject: [PATCH 4/4] updated after review --- lib/bap/bap_log.ml | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/bap/bap_log.ml b/lib/bap/bap_log.ml index cd42fe88d..f71e7f9fa 100644 --- a/lib/bap/bap_log.ml +++ b/lib/bap/bap_log.ml @@ -54,8 +54,12 @@ let print_message ppf msg = | Error -> eprintf "%s@\n%!" msg.message | _ -> () -let lock () = - let file = Filename.get_temp_dir_name () / "bap_lock" in +let lock_filename logdir = + let digest = Md5.digest_string logdir in + let name = sprintf "bap-%s.lock" (Md5.to_hex digest) in + Filename.get_temp_dir_name () / name + +let lock file = let lock = Unix.openfile file Unix.[O_RDWR; O_CREAT] 0o666 in Unix.lockf lock Unix.F_LOCK 0; lock @@ -68,7 +72,7 @@ let open_log_channel user_dir = try let log_folder = log_folder user_dir in let file = log_folder / "log" in - let lock = lock () in + let lock = lock (lock_filename log_folder) in protect ~f:(fun () -> mkdir log_folder; if Sys.file_exists file