akvo / akvo-lumen

Make sense of your data
https://akvo.org/akvo-lumen
GNU Affero General Public License v3.0
63 stars 18 forks source link

Review the use of read-string and eval #2417

Closed iperdomo closed 4 years ago

iperdomo commented 5 years ago

Context

Code cleanup, technical debt, correctness

Problem or idea

There are places where we use read-string for creating objects e.g. https://github.com/akvo/akvo-lumen/blob/4fd9e7c168a4cf4a24fcd466503ed7a0d351d895/backend/specs/akvo/lumen/specs.clj#L34 In this case we could use UUID/fromString for the same result. read-string can execute code.

Note that read-string can execute code (controlled by read-eval), and as such should be used only with trusted sources. https://clojure.github.io/clojure/clojure.core-api.html#clojure.core/read-string

In other places we have (eval) https://github.com/akvo/akvo-lumen/blob/4fd9e7c168a4cf4a24fcd466503ed7a0d351d895/backend/src/akvo/lumen/lib/transformation/derive_category.clj#L39-L40

While not entirely sure the context/meaning of this code snippet, it seems that we want to apply a function, where the function name and parameters are dynamic?

Solution or next step

Revisit the use of read-string and eval

iperdomo commented 5 years ago

Note sure if this make sense at all:

  (let [a 1 b 2 c '+
        exp (read-string (format "(%s %s %s)" c b a))
        res (eval exp)]
    res)
;; => 3

  (let [a 1
        b 2
        c '+
        f (resolve c)]
    (apply f [a b]))
;; => 3
stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.