(CVE-2023-43091) Code injection via service.json file
Original reporter: Michael Evans
Area: Application
Message
Good day,
I am reporting a potential security issue in the GNOME Maps application. The issue is JavaScript code injection and remote code execution. The issue was introduced in merge request 227 (!227 (merged)) and is present in the main branch at the time of writing and since the v43.* series of releases from v43.0 up to and including v45.beta. I believe the issue is not too severe for some reasons listed below.
In the src/transitRouter.js
file, there are uses of the JavaScript eval function that are run on strings coming from a JSON configuration file. The eval function is used to create instances of transit plugin classed which are specified in the transitProviders[].provider.plugin
fields in the JSON configuration file. The value of the field is unchecked and so it is possible to insert arbitrary JavaScript code in the JSON field and have the application execute it when the plugin is loaded.
The JSON file is typically fetched from the URL https://static.gnome.org/gis.gnome.org/v1/service.json. The file can also be read from the local filesystem when the MAPS_DEFAULT_SERVICE
or the MAPS_SERVICE
environment variables are set.
I was able to exploit the issue by providing my own local configuration file using the MAPS_DEFAULT_SERVICE
environment variable. Additionally, I was able to proxy the traffic of the v44.2 Flatpak from Flathub, using mitmproxy and a self-signed root certificate installed into my machine's certificate store, and modify the JSON response to exploit the issue.
A proof of concept configuration file is provided in the patch below. Building and running the application with the patch applied will cause "asdf" to be printed to stdout when performing a public transport route search in South Africa.
diff --git a/data/maps-service.json b/data/maps-service.json
index 12fcbca8..841e6ef5 100644
--- a/data/maps-service.json
+++ b/data/maps-service.json
@@ -34,5 +34,21 @@
"attributionUrl": "https://graphhopper.com/",
"apiKey": "VCIHrHj0pDKb8INLpT4s5hVadNmJ1Q3vi0J4nJYP",
"supportedLanguages": ["de", "en", "fr"]
- }
+ },
+ "transitProviders": [
+ {
+ "provider": {
+ "name": "South Africa",
+ "plugin": "GoMetro();console.log('asdf');//",
+ "attribution": "GoMetro",
+ "attributionUrl": "https://proserver.gometro.co.za/api/v1/docs/#multimodal-routing",
+ "priority": 10,
+ "areas": [
+ {
+ "countries": [ "ZA" ]
+ }
+ ]
+ }
+ }
+ ]
}
diff --git a/org.gnome.Maps.json b/org.gnome.Maps.json
index 5cd90c93..65f4903b 100644
--- a/org.gnome.Maps.json
+++ b/org.gnome.Maps.json
@@ -16,7 +16,8 @@
"--share=network",
"--metadata=X-DConf=migrate-path=/org/gnome/Maps",
"--env=G_ENABLE_DIAGNOSTICS=1",
- "--env=MAPS_DEBUG=1"
+ "--env=MAPS_DEBUG=1",
+ "--env=MAPS_DEFAULT_SERVICE=1"
],
"cleanup" : [
"/include",
I believe this issue is not too severe. If the file provided at the URL is securely controlled by GNOME, which I assume it is, then I hope we can trust there will be no malicious JSON files served that will exploit this issue. Additionally, as the file is fetched over HTTPS, man-in-the-middle attacks are unlikely. Lastly, if a user supplies their own configuration file by using the environment variables, they will be under control of the file's content. However I still believe the code should be changed. This behaviour was likely unintended.
A suggested fix is to create an allowlist for the plugin values that correspond to the plugin classes that can be instantiated. A patch implementing this is provided below.
diff --git a/src/transitRouter.js b/src/transitRouter.js
index 05e6627b..ee606022 100644
--- a/src/transitRouter.js
+++ b/src/transitRouter.js
@@ -32,6 +32,7 @@ import {OpendataCH} from './transitplugins/opendataCH.js';
import {OpenTripPlanner} from './transitplugins/openTripPlanner.js';
import {Resrobot} from './transitplugins/resrobot.js';
+const ALL_PLUGINS = ["GoMetro", "OpendataCH", "OpenTripPlanner", "Resrobot"];
/**
* Class responsible for delegating requests to perform routing in transit
@@ -67,8 +68,7 @@ export class TransitRouter {
// override plugin was specified, try instanciating if not done yet
if (!this._currPluginInstance) {
try {
- this._currentPluginInstance =
- eval(`new ${pluginOverride}()`);
+ this._currentPluginInstance = this._instantiatePlugin(pluginOverride);
} catch (e) {
Utils.debug('Unable to instanciate plugin: ' + pluginOverride);
throw e;
@@ -230,9 +230,7 @@ export class TransitRouter {
try {
let params = provider.params;
- let instance =
- params ? eval(`new ${plugin}(params)`):
- eval(`new ${plugin}()`);
+ let instance = this._instantiatePlugin(plugin, params);
this._providerCache[provider.name] = instance;
@@ -263,4 +261,12 @@ export class TransitRouter {
else
return 0;
}
+
+ _instantiatePlugin(plugin, params) {
+ if (!ALL_PLUGINS.includes(plugin))
+ throw 'Unknown plugin: ' + plugin;
+ return params
+ ? eval(`new ${plugin}(params)`)
+ : eval(`new ${plugin}()`);
+ }
};
Kind regards, Michael Evans