addGeotiff API development

[tim-salabim]

In #25 I've been keeping track of the development for the new js functionality to add geotiff data to a leaflet map. The API has changed enough so that most examples over there won't work anymore. Hence, dev-tracking will continue here.

The "new" API has a few advantages over the old one:

• everything is happening in addGeotiff as opposed to addGeoraster
• we now accept user defined functions in the arith argument as opposed to some string representation
• to save space we only attach the layers necessary for executing arith
• thanks to @yeedle (?) and @alandipert we can now also utilise javascript Math. functions (https://twitter.com/TimSalabim3/status/1301078413315317761) though we are still using the evil eval in one place... to be addressed
• the bounds of the value domain to be mapped to colors is also calculated in the browser. This is currently implemented as the "theoretical" domain, meaning that we calculate the theoretical bounds of possible values using all combinations of minimum/maximum values per band. There is no guarantee that any of these combinations are actually present in the data, but at least we are sure that nothing will lie beyond these bounds. One can always set the domain manually
• for RGB images we have a new R function called addRGB (for both stars and raster objects). We set rgb to TRUE in addGeotiff and the specified bands will be used to create a RGB image (again, this happens in the browser - no pre-calculation in R)

Examples to follow...

[tim-salabim]

The following example currently works, but only with the evil eval in https://github.com/r-spatial/leafem/blob/master/inst/htmlwidgets/lib/georaster-for-leaflet/georaster-binding.js#L104
If I change this line to vals = evalMath(arith); all pixel calculations return 0 as both values[1] and values[0] have the value 255. This is a remnant of the previous call to evalMath in evalDomain here where the latest combination in the for loop evaluates to [255, 255]. evalMath is defined here.

There is some scope creep happening somewhere, but I am not sure where and how to avoid it. Maybe @yeedle has an idea?

library(stars)
library(leaflet)
library(leafem)

tif = system.file("tif/L7_ETMs.tif", package = "stars")
(x1 = read_stars(tif))
strs = st_warp(x1, crs = 4326)
fl = tempfile(fileext = ".tif")
write_stars(strs, fl)

pal = grDevices::colorRampPalette(hcl.colors(9, "viridis"))

ndvi = function(x) (x[4] - x[3]) / (x[4] + x[3])

leaflet() %>%
  addTiles() %>%
  addGeotiff(
    file = fl
    , group = "stars"
    , resolution = 96
    , arith = ndvi
    , colorOptions = colorOptions(
      palette = pal
    )
  )

To get this to work:

remotes::install_github("r-spatial/leafem")

[yeedle]

I've figured out the issue and have a suggested fix (see below), but I want to clear up a couple of things. eval is "evil" because it evaluates user-supplied strings in a privileged context. For example, if you run eval(userSuppliedString) on your server, and the user passes ("exec('rm -rf /')", all your server files will be deleted. Function is just as dangerous as it can accomplish the exact same thing. However, In context of this package (which is a user running eval in their own browser which is sandboxed in the first place), any real danger, like a user accidentally or maliciously running harmful code, is honestly minimal to non-existent. But eval (and Function) can still be a footgun if the user does pass in some wonky string that then causes the package to behave in unexpected ways. This is actually the type of danger for which Function is less "dangerous" than eval - and it is this exact same reason that is causing the issue you are describing.

The difference between eval and Function is that eval has access to local scope which means it can reassign local variables. On the other hand, Function only has access to global scope, making it impossible to mess up local variables. In evalDomain, the values variable is in global scope. The function has access to it and can operate on it. However, when you use it inside the pixelValuesToColorFn function, you want it to access the local values variable, but since it's a Function call and not an eval call, it still can only access the global values which is set to the last value it was set to inside the evalDomain loop.

The solution is to let the evalMath function accept a values argument, and then pass in the values whenever you call evalMath. This would look as follows (btw, I suggest you use something like _values or something similarly obfuscated so that if other code uses values as a variable in global scope it shouldn't clash with this code):

function evalMath(a, values) {
    return Function('values', 'with(Math) return ' + a)(values);
}

When you call evalMath pass in both the user's string and the values argument, e.g.;

pixelValuesToColorFn = values => {
            let vals;
            if (arith === null) {
              if (bands.length > 1) {
                bands = bands[0];
              }
              vals = values[bands];
            }
            if (arith !== null) {
              vals = evalMath(arith, values);
            }
            let clr = scale.domain(domain);
            if (isNaN(vals)) return nacol;
            return clr(vals).hex();
          };

and:

function evalDomain(arr, arith) {
  var out = [];
  for (let i = 0; i < arr.length; i++) {
    values = arr[i];
    out.push(evalMath(arith, values));
  }
  return [Math.min(...out), Math.max(...out)];
}

I hope you find the above helpful!

[tim-salabim]

@yeedle many many thanks! This works like a charm! I've included you as a contributor to the package (if that's ok with you).

For example, if you run eval(userSuppliedString) on your server, and the user passes ("exec('rm -rf /')", all your server files will be deleted. Function is just as dangerous as it can accomplish the exact same thing. However, In context of this package (which is a user running eval in their own browser which is sandboxed in the first place), any real danger, like a user accidentally or maliciously running harmful code, is honestly minimal to non-existent.

Does this mean that if someone is hosting a map that includes eval or Function that it can potentially be hijacked by someone to ingest someEvilString and cause a lot of trouble? Or is this an unlikely scenario?

[yeedle]

I think any case in which you execute user-supplied strings opens you up to some vulnerabilities. Personally I believe that in this particular case it's highly unlikely. Even in the case of a hosted map, the user is still running the javascript in their own sandboxed browser. The only exception I can think of is a case where the javascript will get executed in a headless environment on a server, but I think the way you deparse and translate the R function into a JS expression makes even such a scenario very hard to exploit. If you want to completely stay away from any such concern, I'd suggest you use math.js's math expression parser or expr-eval instead.

[tim-salabim]

Thanks again @yeedle for a great explanation! And especially for the pointer to expr-eval. Do you have any experience with it? It looks like it could be very useful here.

[yeedle]

Don't have much experience but usage is pretty straightforward. You have your expression as a string, and then you have your variable. You instantiate a parser const parser = new Parser(), and pass the string to the parser (const expression = passer.parse(stringExpression)). You can then use the expression instance to evaluate different values with it expression.evaluate(obj)

If you want I can open a PR replacing Function with expr-eval.

[tim-salabim]

Thanks @yeedle ! A PR would be fantastic

[yeedle]

hmm. Just noticed expr-eval desn't avoid security issues either (see here, and the same goes for math.js as well. However, math.js has a fairly straightforward guide how to almost completely eliminate the security risk. Should I do a PR with math.js rather?

[tim-salabim]

Yeah, the safer the better. Especially as I still quite comprehend the potential risks. So better be safe than sorry I guess.

[trafficonese]

The examples using a pixelValuesToColorFn = myCustomJSFunc in #25 dont work for me.
colorOptions doesnt exist on the jS side or is undefined, so colorOptions.palette cannot be read.
Am I missing something?