Skip to content
Permalink
Browse files Browse the repository at this point in the history
Merge pull request from GHSA-w698-693g-23hv
* fix arbitrary code execution vulnerability

Signed-off-by: Jan N. Klug <jan.n.klug@rub.de>

* Update bundles/org.openhab.binding.exec/src/main/java/org/openhab/binding/exec/internal/handler/ExecHandler.java

Co-Authored-By: Christoph Weitkamp <github@christophweitkamp.de>

* address review comments

Signed-off-by: Jan N. Klug <jan.n.klug@rub.de>

Co-authored-by: Christoph Weitkamp <github@christophweitkamp.de>
  • Loading branch information
J-N-K and cweitkamp committed Feb 19, 2020
1 parent c23710a commit 4c4cb66
Show file tree
Hide file tree
Showing 9 changed files with 397 additions and 195 deletions.
13 changes: 10 additions & 3 deletions bundles/org.openhab.binding.exec/README.md
Expand Up @@ -8,8 +8,17 @@ Currently, the binding supports a single type of Thing, being the `command` Thin

## Binding Configuration

The binding does not require any specific configuration.
For security reasons all commands need to be whitelisted.
Allowed commands need to be added to the `misc/exec.whitelist` file in the configuration directory.
Every command needs to be on a separate line.

Example:

```shell
/bin/echo "Hello world!"
/usr/local/bin/apcaccess status
php ./configurations/scripts/script.php %2$s
```

**Linux:**
Note that the commands are executed in the context and with the privileges of the process running the Java Virtual Machine.
Expand All @@ -27,7 +36,6 @@ It is not advised to run the virtual machine as superuser/root.
The "command" Thing requires the command to execute on the shell.
Optionally one can specify:


- `transform` - A [transformation](https://www.openhab.org/docs/configuration/transformations.html) to apply on the execution result string.
- `interval` - An interval, in seconds, the command will be repeatedly executed. Default is 60 seconds, set to 0 to avoid automatic repetition.
- `timeout` - A time-out, in seconds, the execution of the command will time out, and lastly,
Expand All @@ -39,7 +47,6 @@ For each shell command, a separate Thing has to be defined.
Thing exec:command:uniquename [command="/command/to/execute here", interval=15, timeout=5, autorun=false]
```


The `command` itself can be enhanced using the well known syntax of the [Java formatter class syntax](https://docs.oracle.com/javase/8/docs/api/java/util/Formatter.html#syntax).
The following parameters are automatically added:

Expand Down
Expand Up @@ -13,17 +13,19 @@
package org.openhab.binding.exec.internal;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.smarthome.config.core.ConfigConstants;
import org.eclipse.smarthome.core.thing.ThingTypeUID;

import java.io.File;

/**
* The {@link ExecBinding} class defines common constants, which are
* The {@link ExecBindingConstants} class defines common constants, which are
* used across the whole binding.
*
* @author Karel Goderis - Initial contribution
*/
@NonNullByDefault
public class ExecBindingConstants {

public static final String BINDING_ID = "exec";

// List of all Thing Type UIDs
Expand All @@ -35,5 +37,4 @@ public class ExecBindingConstants {
public static final String EXIT = "exit";
public static final String RUN = "run";
public static final String LAST_EXECUTION = "lastexecution";

}
Expand Up @@ -12,11 +12,6 @@
*/
package org.openhab.binding.exec.internal;

import static org.openhab.binding.exec.internal.ExecBindingConstants.THING_COMMAND;

import java.util.Collections;
import java.util.Set;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.eclipse.smarthome.core.thing.Thing;
Expand All @@ -25,7 +20,16 @@
import org.eclipse.smarthome.core.thing.binding.ThingHandler;
import org.eclipse.smarthome.core.thing.binding.ThingHandlerFactory;
import org.openhab.binding.exec.internal.handler.ExecHandler;
import org.osgi.service.component.annotations.Activate;
import org.osgi.service.component.annotations.Component;
import org.osgi.service.component.annotations.Reference;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.Collections;
import java.util.Set;

import static org.openhab.binding.exec.internal.ExecBindingConstants.THING_COMMAND;

/**
* The {@link ExecHandlerFactory} is responsible for creating things and thing
Expand All @@ -36,8 +40,14 @@
@NonNullByDefault
@Component(service = ThingHandlerFactory.class, configurationPid = "binding.exec")
public class ExecHandlerFactory extends BaseThingHandlerFactory {

private static final Set<ThingTypeUID> SUPPORTED_THING_TYPES_UIDS = Collections.singleton(THING_COMMAND);
private final Logger logger = LoggerFactory.getLogger(ExecHandlerFactory.class);
private final ExecWhitelistWatchService execWhitelistWatchService;

@Activate
public ExecHandlerFactory(@Reference ExecWhitelistWatchService execWhitelistWatchService) {
this.execWhitelistWatchService = execWhitelistWatchService;
}

@Override
public boolean supportsThingType(ThingTypeUID thingTypeUID) {
Expand All @@ -49,7 +59,7 @@ public boolean supportsThingType(ThingTypeUID thingTypeUID) {
ThingTypeUID thingTypeUID = thing.getThingTypeUID();

if (thingTypeUID.equals(THING_COMMAND)) {
return new ExecHandler(thing);
return new ExecHandler(thing, execWhitelistWatchService);
}

return null;
Expand Down
@@ -0,0 +1,81 @@
/**
* Copyright (c) 2010-2020 Contributors to the openHAB project
*
* See the NOTICE file(s) distributed with this work for additional
* information.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0
*
* SPDX-License-Identifier: EPL-2.0
*/
package org.openhab.binding.exec.internal;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.eclipse.smarthome.config.core.ConfigConstants;
import org.eclipse.smarthome.core.service.AbstractWatchService;
import org.osgi.service.component.annotations.Activate;
import org.osgi.service.component.annotations.Component;

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.WatchEvent;
import java.util.HashSet;
import java.util.Set;

import static java.nio.file.StandardWatchEventKinds.*;

/**
* The {@link ExecWhitelistWatchService} provides a whitelist check for exec commands
*
* @author Jan N. Klug - Initial contribution
*/
@Component(service = ExecWhitelistWatchService.class)
@NonNullByDefault
public class ExecWhitelistWatchService extends AbstractWatchService {
private static final String COMMAND_WHITELIST_PATH = ConfigConstants.getConfigFolder() + File.separator + "misc";
private static final String COMMAND_WHITELIST_FILE = "exec.whitelist";
private final Set<String> commandWhitelist = new HashSet<>();

@Activate
public ExecWhitelistWatchService() {
super(COMMAND_WHITELIST_PATH);
}

@Override
protected boolean watchSubDirectories() {
return false;
}

@Override
protected WatchEvent.Kind<?>[] getWatchEventKinds(@Nullable Path directory) {
return new WatchEvent.Kind<?>[] { ENTRY_CREATE, ENTRY_DELETE, ENTRY_MODIFY };
}

@Override
protected void processWatchEvent(@Nullable WatchEvent<?> event, WatchEvent.@Nullable Kind<?> kind, @Nullable Path path) {
if (path.endsWith(COMMAND_WHITELIST_FILE)) {
commandWhitelist.clear();
try {
Files.lines(path).forEach(commandWhitelist::add);
logger.debug("Updated command whitelist: {}", commandWhitelist);
} catch (IOException e) {
logger.warn("Cannot read whitelist file, exec binding commands won't be processed: {}", e.getMessage());
}
}
}

/**
* Check if a command is whitelisted
*
* @param command the command to check alias
* @return true if whitelisted, false if not
*/
public boolean isWhitelisted(String command) {
return commandWhitelist.contains(command);
}
}

0 comments on commit 4c4cb66

Please sign in to comment.