Skip to content

Commit 6a421d5

Browse files
anfenganfeng
committed
check file download path for security
1 parent dd499ff commit 6a421d5

File tree

2 files changed

+54
-36
lines changed

2 files changed

+54
-36
lines changed

‎storm-core/src/clj/backtype/storm/daemon/nimbus.clj

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -881,6 +881,17 @@
881881
(throw (InvalidTopologyException.
882882
(str "Topology name cannot contain any of the following: " (pr-str DISALLOWED-TOPOLOGY-NAME-STRS))))))
883883

884+
(defn check-file-location! [conf file-path]
885+
(log-debug "check file location:" file-path)
886+
(if (clojure.string/blank? file-path)
887+
(throw (AuthorizationException. (str "Invalid file path:" file-path))))
888+
(let [stormdist-dir (str (master-stormdist-root conf) "/")
889+
stormdist-dir-len (.length stormdist-dir)
890+
file-path-len (.length file-path)
891+
file-path-prefix (if (> file-path-len stormdist-dir-len) (.substring file-path 0 stormdist-dir-len))]
892+
(if (not= (compare stormdist-dir file-path-prefix) 0)
893+
(throw (AuthorizationException. (str "Invalid file path:" file-path))))))
894+
884895
(defn- try-read-storm-conf [conf storm-id]
885896
(try-cause
886897
(read-storm-conf conf storm-id)
@@ -1084,6 +1095,7 @@
10841095
))
10851096

10861097
(^String beginFileDownload [this ^String file]
1098+
(check-file-location! (:conf nimbus) file)
10871099
(check-authorization! nimbus nil nil "fileDownload")
10881100
(let [is (BufferFileInputStream. file)
10891101
id (uuid)]

‎storm-core/test/clj/backtype/storm/nimbus_test.clj

Lines changed: 42 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,13 @@
146146
(is (not-nil? ((:executor->start-time-secs assignment) e))))
147147
))
148148

149-
149+
(deftest test-file-bogus-bogus-download
150+
(with-local-cluster [cluster :daemon-conf {SUPERVISOR-ENABLE false TOPOLOGY-ACKER-EXECUTORS 0}]
151+
(let [nimbus (:nimbus cluster)]
152+
(is (thrown? AuthorizationException (.beginFileDownload nimbus nil)))
153+
(is (thrown? AuthorizationException (.beginFileDownload nimbus "")))
154+
(is (thrown? AuthorizationException (.beginFileDownload nimbus "/bogus-path/foo")))
155+
)))
150156

151157
(deftest test-bogusId
152158
(with-local-cluster [cluster :supervisors 4 :ports-per-supervisor 3 :daemon-conf {SUPERVISOR-ENABLE false TOPOLOGY-ACKER-EXECUTORS 0}]
@@ -228,7 +234,7 @@
228234
{"1" (thrift/mk-spout-spec (TestPlannerSpout. false) :parallelism-hint 3)}
229235
{"2" (thrift/mk-bolt-spec {"1" :none} (TestPlannerBolt.) :parallelism-hint 5)
230236
"3" (thrift/mk-bolt-spec {"2" :none} (TestPlannerBolt.))}))
231-
237+
232238
(submit-local-topology nimbus "noniso" {TOPOLOGY-OPTIMIZE false TOPOLOGY-WORKERS 4} topology)
233239
(advance-cluster-time cluster 1)
234240
(is (= 4 (topology-num-nodes state "noniso")))
@@ -237,32 +243,32 @@
237243
(submit-local-topology nimbus "tester1" {TOPOLOGY-OPTIMIZE false TOPOLOGY-WORKERS 6} topology)
238244
(submit-local-topology nimbus "tester2" {TOPOLOGY-OPTIMIZE false TOPOLOGY-WORKERS 6} topology)
239245
(advance-cluster-time cluster 1)
240-
246+
241247
(bind task-info-tester1 (storm-component->task-info cluster "tester1"))
242248
(bind task-info-tester2 (storm-component->task-info cluster "tester2"))
243-
249+
244250

245251
(is (= 1 (topology-num-nodes state "noniso")))
246252
(is (= 3 (storm-num-workers state "noniso")))
247253

248254
(is (= {2 3} (topology-node-distribution state "tester1")))
249255
(is (= {3 2} (topology-node-distribution state "tester2")))
250-
256+
251257
(is (apply disjoint? (map (partial topology-nodes state) ["noniso" "tester1" "tester2"])))
252-
258+
253259
(check-consistency cluster "tester1")
254260
(check-consistency cluster "tester2")
255261
(check-consistency cluster "noniso")
256262

257263
;;check that nothing gets reassigned
258264
(bind tester1-slots (topology-slots state "tester1"))
259265
(bind tester2-slots (topology-slots state "tester2"))
260-
(bind noniso-slots (topology-slots state "noniso"))
266+
(bind noniso-slots (topology-slots state "noniso"))
261267
(advance-cluster-time cluster 20)
262268
(is (= tester1-slots (topology-slots state "tester1")))
263269
(is (= tester2-slots (topology-slots state "tester2")))
264270
(is (= noniso-slots (topology-slots state "noniso")))
265-
271+
266272
)))
267273

268274
(deftest test-zero-executor-or-tasks
@@ -296,7 +302,7 @@
296302
(check-consistency cluster "mystorm")
297303
(is (= 5 (count (task-info "1"))))
298304
(check-distribution (executor-info "1") [2 2 1])
299-
305+
300306
(is (= 2 (count (task-info "2"))))
301307
(check-distribution (executor-info "2") [1 1])
302308

@@ -369,13 +375,13 @@
369375
(is (thrown? AlreadyAliveException (submit-local-topology (:nimbus cluster) "2test" {} topology)))
370376
(advance-cluster-time cluster 5)
371377
(is (= 1 (count (.heartbeat-storms state))))
372-
378+
373379
(advance-cluster-time cluster 6)
374380
(is (nil? (.storm-base state storm-id nil)))
375381
(is (nil? (.assignment-info state storm-id nil)))
376382
(advance-cluster-time cluster 11)
377383
(is (= 0 (count (.heartbeat-storms state))))
378-
384+
379385
(submit-local-topology (:nimbus cluster) "test3" {TOPOLOGY-MESSAGE-TIMEOUT-SECS 5} topology)
380386
(bind storm-id3 (get-storm-id state "test3"))
381387
(advance-cluster-time cluster 1)
@@ -389,12 +395,12 @@
389395
;; this guarantees that monitor thread won't trigger for 10 more seconds
390396
(advance-time-secs! 11)
391397
(wait-until-cluster-waiting cluster)
392-
398+
393399
(submit-local-topology (:nimbus cluster) "test3" {TOPOLOGY-MESSAGE-TIMEOUT-SECS 5} topology)
394400
(bind storm-id3 (get-storm-id state "test3"))
395401

396402
(bind executor-id (first (topology-executors cluster storm-id3)))
397-
403+
398404
(do-executor-heartbeat cluster storm-id3 executor-id)
399405

400406
(.killTopology (:nimbus cluster) "test3")
@@ -410,7 +416,7 @@
410416
(advance-cluster-time cluster 9)
411417
(is (not-nil? (.assignment-info state storm-id4 nil)))
412418
(advance-cluster-time cluster 2)
413-
(is (nil? (.assignment-info state storm-id4 nil)))
419+
(is (nil? (.assignment-info state storm-id4 nil)))
414420
)))
415421

416422
(deftest test-reassignment
@@ -434,7 +440,7 @@
434440
(bind [executor-id1 executor-id2] (topology-executors cluster storm-id))
435441
(bind ass1 (executor-assignment cluster storm-id executor-id1))
436442
(bind ass2 (executor-assignment cluster storm-id executor-id2))
437-
443+
438444
(advance-cluster-time cluster 59)
439445
(do-executor-heartbeat cluster storm-id executor-id1)
440446
(do-executor-heartbeat cluster storm-id executor-id2)
@@ -455,7 +461,7 @@
455461
(do-executor-heartbeat cluster storm-id executor-id1)
456462
(is (= ass1 (executor-assignment cluster storm-id executor-id1)))
457463
(check-consistency cluster "test")
458-
464+
459465
(advance-cluster-time cluster 11)
460466
(is (= ass1 (executor-assignment cluster storm-id executor-id1)))
461467
(is (not= ass2 (executor-assignment cluster storm-id executor-id2)))
@@ -531,7 +537,7 @@
531537
(bind [executor-id1 executor-id2] (topology-executors cluster storm-id))
532538
(bind ass1 (executor-assignment cluster storm-id executor-id1))
533539
(bind ass2 (executor-assignment cluster storm-id executor-id2))
534-
540+
535541
(advance-cluster-time cluster 59)
536542
(do-executor-heartbeat cluster storm-id executor-id1)
537543
(do-executor-heartbeat cluster storm-id executor-id2)
@@ -603,7 +609,7 @@
603609
(bind common (first (find-first (fn [[k v]] (= 3 (count v))) slot-executors2)))
604610
(is (not-nil? common))
605611
(is (= (slot-executors2 common) (slot-executors common)))
606-
612+
607613
;; check that start times are changed for everything but the common one
608614
(bind same-executors (slot-executors2 common))
609615
(bind changed-executors (apply concat (vals (dissoc slot-executors2 common))))
@@ -649,7 +655,7 @@
649655
(bind slot-executors (slot-assignments cluster storm-id))
650656
(check-executor-distribution slot-executors [1 1 1])
651657
(check-num-nodes slot-executors 3)
652-
658+
653659
(is (thrown? InvalidTopologyException
654660
(.rebalance (:nimbus cluster) "test"
655661
(doto (RebalanceOptions.)
@@ -679,7 +685,7 @@
679685
(slot-assignments cluster storm-id)
680686
distribution)))
681687
(checker [2 2 2])
682-
688+
683689
(.rebalance (:nimbus cluster) "test"
684690
(doto (RebalanceOptions.)
685691
(.set_num_workers 6)
@@ -688,7 +694,7 @@
688694
(checker [2 2 2])
689695
(advance-cluster-time cluster 3)
690696
(checker [1 1 1 1 1 1])
691-
697+
692698
(.rebalance (:nimbus cluster) "test"
693699
(doto (RebalanceOptions.)
694700
(.set_num_executors {"1" 1})
@@ -706,11 +712,11 @@
706712
(advance-cluster-time cluster 32)
707713
(checker [2 2 2 2])
708714
(check-consistency cluster "test")
709-
715+
710716
(bind executor-info (->> (storm-component->executor-info cluster "test")
711717
(map-val #(map executor-id->tasks %))))
712718
(check-distribution (executor-info "1") [2 2 2 2 1 1 1 1])
713-
719+
714720
)))
715721

716722
(deftest test-submit-invalid
@@ -723,13 +729,13 @@
723729
(bind topology (thrift/mk-topology
724730
{"1" (thrift/mk-spout-spec (TestPlannerSpout. true) :parallelism-hint 0 :conf {TOPOLOGY-TASKS 1})}
725731
{}))
726-
732+
727733
(is (thrown? InvalidTopologyException
728734
(submit-local-topology (:nimbus cluster)
729735
"test"
730736
{}
731737
topology)))
732-
738+
733739
(bind topology (thrift/mk-topology
734740
{"1" (thrift/mk-spout-spec (TestPlannerSpout. true) :parallelism-hint 1 :conf {TOPOLOGY-TASKS 1})}
735741
{}))
@@ -747,7 +753,7 @@
747753
(is (thrown? InvalidTopologyException
748754
(submit-local-topology (:nimbus cluster)
749755
"test"
750-
{TOPOLOGY-WORKERS 3}
756+
{TOPOLOGY-WORKERS 3}
751757
topology)))
752758
(bind topology (thrift/mk-topology
753759
{"1" (thrift/mk-spout-spec (TestPlannerSpout. true)
@@ -832,32 +838,32 @@
832838
))))
833839

834840
(deftest test-nimbus-iface-submitTopologyWithOpts-checks-authorization
835-
(with-local-cluster [cluster
836-
:daemon-conf {NIMBUS-AUTHORIZER
841+
(with-local-cluster [cluster
842+
:daemon-conf {NIMBUS-AUTHORIZER
837843
"backtype.storm.security.auth.authorizer.DenyAuthorizer"}]
838844
(let [
839845
nimbus (:nimbus cluster)
840846
topology (thrift/mk-topology {} {})
841847
]
842848
(is (thrown? AuthorizationException
843-
(submit-local-topology-with-opts nimbus "mystorm" {} topology
849+
(submit-local-topology-with-opts nimbus "mystorm" {} topology
844850
(SubmitOptions. TopologyInitialStatus/INACTIVE))
845851
))
846852
)
847853
)
848854
)
849855

850856
(deftest test-nimbus-iface-methods-check-authorization
851-
(with-local-cluster [cluster
852-
:daemon-conf {NIMBUS-AUTHORIZER
857+
(with-local-cluster [cluster
858+
:daemon-conf {NIMBUS-AUTHORIZER
853859
"backtype.storm.security.auth.authorizer.DenyAuthorizer"}]
854860
(let [
855861
nimbus (:nimbus cluster)
856862
topology (thrift/mk-topology {} {})
857863
]
858864
; Fake good authorization as part of setup.
859865
(mocking [nimbus/check-authorization!]
860-
(submit-local-topology-with-opts nimbus "test" {} topology
866+
(submit-local-topology-with-opts nimbus "test" {} topology
861867
(SubmitOptions. TopologyInitialStatus/INACTIVE))
862868
)
863869
(stubbing [nimbus/storm-active? true]
@@ -933,16 +939,16 @@
933939
(stubbing [topology-bases bogus-bases]
934940
(let [topos (.get_topologies (.getClusterInfo nimbus))]
935941
; The number of topologies in the summary is correct.
936-
(is (= (count
942+
(is (= (count
937943
(filter (fn [b] (second b)) bogus-bases)) (count topos)))
938944
; Each topology present has a valid name.
939945
(is (empty?
940946
(filter (fn [t] (or (nil? t) (nil? (.get_name t)))) topos)))
941947
; The topologies are those with valid bases.
942948
(is (empty?
943-
(filter (fn [t]
944-
(or
945-
(nil? t)
949+
(filter (fn [t]
950+
(or
951+
(nil? t)
946952
(not (number? (read-string (.get_id t))))
947953
(odd? (read-string (.get_id t)))
948954
)) topos)))

0 commit comments

Comments
 (0)