From 65b5a307fc1c55a197ee337180842ac6885ba784 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Tue, 21 Feb 2012 15:06:49 +0000
Subject: [PATCH] Prevent users from setting JOBSERIAL and from setting
 variables with invalid names.

---
 daemon/daemon.ml  | 33 ++++++++++++++++++++++++++++-----
 lib/whenproto.x   |  2 +-
 lib/whenutils.ml  |  3 +++
 lib/whenutils.mli |  4 ++++
 tools/whenjobs.ml |  8 +++++++-
 5 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/daemon/daemon.ml b/daemon/daemon.ml
index a8bd514..47a67ed 100644
--- a/daemon/daemon.ml
+++ b/daemon/daemon.ml
@@ -97,12 +97,35 @@ and proc_reload_file () =
 and proc_set_variable (name, value) =
   if !debug then Syslog.notice "remote call: set_variable %s" name;
 
-  let value = variable_of_rpc value in
-  variables := StringMap.add name value !variables;
+  try
+    (* Don't permit certain names. *)
+    if name = "JOBSERIAL" then
+      failwith "JOBSERIAL variable cannot be set";
+
+    let len = String.length name in
+    if len = 0 then
+      failwith "variable name is an empty string";
+    if name.[0] <> '_' && not (isalpha name.[0]) then
+      failwith "variable name must start with alphabetic character or underscore";
+
+    let rec loop i =
+      if i >= len then ()
+      else if name.[i] <> '_' && not (isalnum name.[i]) then
+        failwith "variable name contains non-alphanumeric non-underscore character"
+      else loop (i+1)
+    in
+    loop 1;
+
+    let value = variable_of_rpc value in
+    variables := StringMap.add name value !variables;
+
+    (* Which jobs need to be re-evaluated? *)
+    let jobnames = try StringMap.find name !dependencies with Not_found -> [] in
+    reevaluate_whenjobs jobnames;
 
-  (* Which jobs need to be re-evaluated? *)
-  let jobnames = try StringMap.find name !dependencies with Not_found -> [] in
-  reevaluate_whenjobs jobnames
+    `ok
+  with
+    Failure msg -> `error msg
 
 and proc_get_variable name =
   if !debug then Syslog.notice "remote call: get_variable %s" name;
diff --git a/lib/whenproto.x b/lib/whenproto.x
index 58d3ca2..2e64636 100644
--- a/lib/whenproto.x
+++ b/lib/whenproto.x
@@ -65,7 +65,7 @@ union variable switch (variable_type t) {
 program When {
   version V1 {
     status reload_file (void) = 1;
-    void set_variable (variable_name, variable) = 2;
+    status set_variable (variable_name, variable) = 2;
     variable get_variable (variable_name) = 3;
     variable_name_list get_variable_names (void) = 4;
     status exit_daemon (void) = 5;
diff --git a/lib/whenutils.ml b/lib/whenutils.ml
index 9614ef8..cdfca47 100644
--- a/lib/whenutils.ml
+++ b/lib/whenutils.ml
@@ -41,6 +41,9 @@ module StringSet = Set.Make (String)
 
 let (//) = Filename.concat
 
+let isalpha = function 'a'..'z' | 'A'..'Z' -> true | _ -> false
+let isalnum = function 'a'..'z' | 'A'..'Z' | '0'..'9' -> true | _ -> false
+
 let rec filter_map f = function
   | [] -> []
   | x :: xs ->
diff --git a/lib/whenutils.mli b/lib/whenutils.mli
index f0fb306..73d5084 100644
--- a/lib/whenutils.mli
+++ b/lib/whenutils.mli
@@ -117,6 +117,10 @@ end
 val (//) : string -> string -> string
 (** [dir // file] concatenates directory and file. *)
 
+val isalpha : char -> bool
+val isalnum : char -> bool
+(** Character tests. *)
+
 val filter_map : ('a -> 'b option) -> 'a list -> 'b list
 (** Filter + map. *)
 
diff --git a/tools/whenjobs.ml b/tools/whenjobs.ml
index 0810649..60795ba 100644
--- a/tools/whenjobs.ml
+++ b/tools/whenjobs.ml
@@ -296,7 +296,13 @@ and set_variable name value typ =
       ) in
 
   let client = start_client () in
-  Whenproto_clnt.When.V1.set_variable client (name, value);
+  (match Whenproto_clnt.When.V1.set_variable client (name, value) with
+  | `ok -> ()
+  | `error msg ->
+    eprintf "whenjobs: set: %s\n" msg;
+    suggest_check_server_logs ();
+    exit 1
+  );
   stop_client client
 
 and get_variable name =
-- 
1.8.3.1