From 149275ef2b79b61d11e6b2795a0b8c4c16fdc043 Mon Sep 17 00:00:00 2001 From: ViacheslavKlimov Date: Thu, 29 Sep 2022 15:29:29 +0300 Subject: [PATCH] Limits for JS script body, input args and invocation result size --- .../script/AbstractJsInvokeService.java | 65 +++++++++-- .../service/script/JsInvokeService.java | 2 +- .../script/NashornJsInvokeService.java | 31 ++--- .../service/script/RemoteJsInvokeService.java | 12 ++ .../script/RuleNodeJsScriptEngine.java | 4 +- .../src/main/resources/thingsboard.yml | 6 + .../service/script/JsInvokeServiceTest.java | 106 ++++++++++++++++++ .../service/script/MockJsInvokeService.java | 2 +- 8 files changed, 202 insertions(+), 26 deletions(-) create mode 100644 application/src/test/java/org/thingsboard/server/service/script/JsInvokeServiceTest.java diff --git a/application/src/main/java/org/thingsboard/server/service/script/AbstractJsInvokeService.java b/application/src/main/java/org/thingsboard/server/service/script/AbstractJsInvokeService.java index eef49ae518..c44fa2e335 100644 --- a/application/src/main/java/org/thingsboard/server/service/script/AbstractJsInvokeService.java +++ b/application/src/main/java/org/thingsboard/server/service/script/AbstractJsInvokeService.java @@ -17,6 +17,7 @@ package org.thingsboard.server.service.script; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.MoreExecutors; import lombok.extern.slf4j.Slf4j; import org.thingsboard.common.util.ThingsBoardThreadFactory; import org.thingsboard.server.common.data.ApiUsageRecordKey; @@ -30,9 +31,10 @@ import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicInteger; +import static java.lang.String.format; + /** * Created by ashvayka on 26.09.18. */ @@ -65,33 +67,45 @@ public abstract class AbstractJsInvokeService implements JsInvokeService { @Override public ListenableFuture eval(TenantId tenantId, JsScriptType scriptType, String scriptBody, String... argNames) { if (apiUsageStateService.getApiUsageState(tenantId).isJsExecEnabled()) { + if (scriptBodySizeExceeded(scriptBody)) { + return error(format("Script body exceeds maximum allowed size of %s symbols", getMaxScriptBodySize())); + } UUID scriptId = UUID.randomUUID(); String functionName = "invokeInternal_" + scriptId.toString().replace('-', '_'); String jsScript = generateJsScript(scriptType, functionName, scriptBody, argNames); return doEval(scriptId, functionName, jsScript); } else { - return Futures.immediateFailedFuture(new RuntimeException("JS Execution is disabled due to API limits!")); + return error("JS Execution is disabled due to API limits!"); } } @Override - public ListenableFuture invokeFunction(TenantId tenantId, CustomerId customerId, UUID scriptId, Object... args) { + public ListenableFuture invokeFunction(TenantId tenantId, CustomerId customerId, UUID scriptId, Object... args) { if (apiUsageStateService.getApiUsageState(tenantId).isJsExecEnabled()) { String functionName = scriptIdToNameMap.get(scriptId); if (functionName == null) { - return Futures.immediateFailedFuture(new RuntimeException("No compiled script found for scriptId: [" + scriptId + "]!")); + return error("No compiled script found for scriptId: [" + scriptId + "]!"); } if (!isDisabled(scriptId)) { + if (argsSizeExceeded(args)) { + return scriptExecutionError(scriptId, format("Script input arguments exceed maximum allowed total args size of %s symbols", getMaxTotalArgsSize())); + } apiUsageClient.report(tenantId, customerId, ApiUsageRecordKey.JS_EXEC_COUNT, 1); - return doInvokeFunction(scriptId, functionName, args); + return Futures.transformAsync(doInvokeFunction(scriptId, functionName, args), output -> { + String result = output.toString(); + if (resultSizeExceeded(result)) { + return scriptExecutionError(scriptId, format("Script invocation result exceeds maximum allowed size of %s symbols", getMaxResultSize())); + } + return Futures.immediateFuture(result); + }, MoreExecutors.directExecutor()); } else { String message = "Script invocation is blocked due to maximum error count " + getMaxErrors() + ", scriptId " + scriptId + "!"; log.warn(message); - return Futures.immediateFailedFuture(new RuntimeException(message)); + return error(message); } } else { - return Futures.immediateFailedFuture(new RuntimeException("JS Execution is disabled due to API limits!")); + return error("JS Execution is disabled due to API limits!"); } } @@ -120,6 +134,12 @@ public abstract class AbstractJsInvokeService implements JsInvokeService { protected abstract long getMaxBlacklistDuration(); + protected abstract long getMaxTotalArgsSize(); + + protected abstract long getMaxResultSize(); + + protected abstract long getMaxScriptBodySize(); + protected void onScriptExecutionError(UUID scriptId, Throwable t, String scriptBody) { DisableListInfo disableListInfo = disabledFunctions.computeIfAbsent(scriptId, key -> new DisableListInfo()); log.warn("Script has exception and will increment counter {} on disabledFunctions for id {}, exception {}, cause {}, scriptBody {}", @@ -127,6 +147,27 @@ public abstract class AbstractJsInvokeService implements JsInvokeService { disableListInfo.incrementAndGet(); } + private boolean scriptBodySizeExceeded(String scriptBody) { + if (getMaxScriptBodySize() <= 0) return false; + return scriptBody.length() > getMaxScriptBodySize(); + } + + private boolean argsSizeExceeded(Object[] args) { + if (getMaxTotalArgsSize() <= 0) return false; + long totalArgsSize = 0; + for (Object arg : args) { + if (arg instanceof CharSequence) { + totalArgsSize += ((CharSequence) arg).length(); + } + } + return totalArgsSize > getMaxTotalArgsSize(); + } + + private boolean resultSizeExceeded(String result) { + if (getMaxResultSize() <= 0) return false; + return result.length() > getMaxResultSize(); + } + private String generateJsScript(JsScriptType scriptType, String functionName, String scriptBody, String... argNames) { if (scriptType == JsScriptType.RULE_NODE_SCRIPT) { return RuleNodeScriptFactory.generateRuleNodeScript(functionName, scriptBody, argNames); @@ -148,6 +189,16 @@ public abstract class AbstractJsInvokeService implements JsInvokeService { } } + private ListenableFuture error(String message) { + return Futures.immediateFailedFuture(new RuntimeException(message)); + } + + private ListenableFuture scriptExecutionError(UUID scriptId, String errorMsg) { + RuntimeException error = new RuntimeException(errorMsg); + onScriptExecutionError(scriptId, error, null); + return Futures.immediateFailedFuture(error); + } + private class DisableListInfo { private final AtomicInteger counter; private long expirationTime; diff --git a/application/src/main/java/org/thingsboard/server/service/script/JsInvokeService.java b/application/src/main/java/org/thingsboard/server/service/script/JsInvokeService.java index 3e5d014bef..f9af2f83e4 100644 --- a/application/src/main/java/org/thingsboard/server/service/script/JsInvokeService.java +++ b/application/src/main/java/org/thingsboard/server/service/script/JsInvokeService.java @@ -25,7 +25,7 @@ public interface JsInvokeService { ListenableFuture eval(TenantId tenantId, JsScriptType scriptType, String scriptBody, String... argNames); - ListenableFuture invokeFunction(TenantId tenantId, CustomerId customerId, UUID scriptId, Object... args); + ListenableFuture invokeFunction(TenantId tenantId, CustomerId customerId, UUID scriptId, Object... args); ListenableFuture release(UUID scriptId); diff --git a/application/src/main/java/org/thingsboard/server/service/script/NashornJsInvokeService.java b/application/src/main/java/org/thingsboard/server/service/script/NashornJsInvokeService.java index 5d64017f33..8d794ab24d 100644 --- a/application/src/main/java/org/thingsboard/server/service/script/NashornJsInvokeService.java +++ b/application/src/main/java/org/thingsboard/server/service/script/NashornJsInvokeService.java @@ -15,6 +15,7 @@ */ package org.thingsboard.server.service.script; +import lombok.Getter; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; @@ -32,18 +33,33 @@ public class NashornJsInvokeService extends AbstractNashornJsInvokeService { @Value("${js.local.use_js_sandbox}") private boolean useJsSandbox; + @Getter @Value("${js.local.monitor_thread_pool_size}") private int monitorThreadPoolSize; + @Getter @Value("${js.local.max_cpu_time}") private long maxCpuTime; + @Getter @Value("${js.local.max_errors}") private int maxErrors; @Value("${js.local.max_black_list_duration_sec:60}") private int maxBlackListDurationSec; + @Getter + @Value("${js.local.max_total_args_size:100000}") + private long maxTotalArgsSize; + + @Getter + @Value("${js.local.max_result_size:300000}") + private long maxResultSize; + + @Getter + @Value("${js.local.max_script_body_size:50000}") + private long maxScriptBodySize; + public NashornJsInvokeService(TbApiUsageStateService apiUsageStateService, TbApiUsageClient apiUsageClient, JsExecutorService jsExecutor) { super(apiUsageStateService, apiUsageClient, jsExecutor); } @@ -53,21 +69,6 @@ public class NashornJsInvokeService extends AbstractNashornJsInvokeService { return useJsSandbox; } - @Override - protected int getMonitorThreadPoolSize() { - return monitorThreadPoolSize; - } - - @Override - protected long getMaxCpuTime() { - return maxCpuTime; - } - - @Override - protected int getMaxErrors() { - return maxErrors; - } - @Override protected long getMaxBlacklistDuration() { return TimeUnit.SECONDS.toMillis(maxBlackListDurationSec); diff --git a/application/src/main/java/org/thingsboard/server/service/script/RemoteJsInvokeService.java b/application/src/main/java/org/thingsboard/server/service/script/RemoteJsInvokeService.java index 86b364afd4..12f99b9e12 100644 --- a/application/src/main/java/org/thingsboard/server/service/script/RemoteJsInvokeService.java +++ b/application/src/main/java/org/thingsboard/server/service/script/RemoteJsInvokeService.java @@ -70,6 +70,18 @@ public class RemoteJsInvokeService extends AbstractJsInvokeService { @Value("${js.remote.stats.enabled:false}") private boolean statsEnabled; + @Getter + @Value("${js.remote.max_total_args_size:100000}") + private long maxTotalArgsSize; + + @Getter + @Value("${js.remote.max_result_size:300000}") + private long maxResultSize; + + @Getter + @Value("${js.remote.max_script_body_size:50000}") + private long maxScriptBodySize; + private final AtomicInteger queuePushedMsgs = new AtomicInteger(0); private final AtomicInteger queueInvokeMsgs = new AtomicInteger(0); private final AtomicInteger queueEvalMsgs = new AtomicInteger(0); diff --git a/application/src/main/java/org/thingsboard/server/service/script/RuleNodeJsScriptEngine.java b/application/src/main/java/org/thingsboard/server/service/script/RuleNodeJsScriptEngine.java index c90e104ff4..f1374711d5 100644 --- a/application/src/main/java/org/thingsboard/server/service/script/RuleNodeJsScriptEngine.java +++ b/application/src/main/java/org/thingsboard/server/service/script/RuleNodeJsScriptEngine.java @@ -210,11 +210,11 @@ public class RuleNodeJsScriptEngine implements org.thingsboard.rule.engine.api.S return executeScriptAsync(msg.getCustomerId(), inArgs[0], inArgs[1], inArgs[2]); } - ListenableFuture executeScriptAsync(CustomerId customerId, Object... args) { + ListenableFuture executeScriptAsync(CustomerId customerId, String... args) { return Futures.transformAsync(sandboxService.invokeFunction(tenantId, customerId, this.scriptId, args), o -> { try { - return Futures.immediateFuture(mapper.readTree(o.toString())); + return Futures.immediateFuture(mapper.readTree(o)); } catch (Exception e) { if (e.getCause() instanceof ScriptException) { return Futures.immediateFailedFuture(e.getCause()); diff --git a/application/src/main/resources/thingsboard.yml b/application/src/main/resources/thingsboard.yml index 9edd5d0ed5..3cbb2750a5 100644 --- a/application/src/main/resources/thingsboard.yml +++ b/application/src/main/resources/thingsboard.yml @@ -603,6 +603,9 @@ js: max_requests_timeout: "${LOCAL_JS_MAX_REQUEST_TIMEOUT:0}" # Maximum time in seconds for black listed function to stay in the list. max_black_list_duration_sec: "${LOCAL_JS_SANDBOX_MAX_BLACKLIST_DURATION_SEC:60}" + max_total_args_size: "${LOCAL_JS_SANDBOX_MAX_TOTAL_ARGS_SIZE:100000}" + max_result_size: "${LOCAL_JS_SANDBOX_MAX_RESULT_SIZE:300000}" + max_script_body_size: "${LOCAL_JS_SANDBOX_MAX_SCRIPT_BODY_SIZE:50000}" stats: enabled: "${TB_JS_LOCAL_STATS_ENABLED:false}" print_interval_ms: "${TB_JS_LOCAL_STATS_PRINT_INTERVAL_MS:10000}" @@ -612,6 +615,9 @@ js: max_errors: "${REMOTE_JS_SANDBOX_MAX_ERRORS:3}" # Maximum time in seconds for black listed function to stay in the list. max_black_list_duration_sec: "${REMOTE_JS_SANDBOX_MAX_BLACKLIST_DURATION_SEC:60}" + max_total_args_size: "${REMOTE_JS_SANDBOX_MAX_TOTAL_ARGS_SIZE:100000}" + max_result_size: "${REMOTE_JS_SANDBOX_MAX_RESULT_SIZE:300000}" + max_script_body_size: "${REMOTE_JS_SANDBOX_MAX_SCRIPT_BODY_SIZE:50000}" stats: enabled: "${TB_JS_REMOTE_STATS_ENABLED:false}" print_interval_ms: "${TB_JS_REMOTE_STATS_PRINT_INTERVAL_MS:10000}" diff --git a/application/src/test/java/org/thingsboard/server/service/script/JsInvokeServiceTest.java b/application/src/test/java/org/thingsboard/server/service/script/JsInvokeServiceTest.java new file mode 100644 index 0000000000..d382340a56 --- /dev/null +++ b/application/src/test/java/org/thingsboard/server/service/script/JsInvokeServiceTest.java @@ -0,0 +1,106 @@ +/** + * Copyright © 2016-2022 The Thingsboard Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.thingsboard.server.service.script; + +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.test.context.TestPropertySource; +import org.thingsboard.server.common.data.id.TenantId; +import org.thingsboard.server.controller.AbstractControllerTest; +import org.thingsboard.server.dao.service.DaoSqlTest; + +import java.util.UUID; +import java.util.concurrent.ExecutionException; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; + +@DaoSqlTest +@TestPropertySource(properties = { + "js.local.max_script_body_size=50", + "js.local.max_total_args_size=50", + "js.local.max_result_size=50", + "js.local.max_errors=2" +}) +class JsInvokeServiceTest extends AbstractControllerTest { + + @Autowired + private NashornJsInvokeService jsInvokeService; + + @Value("${js.local.max_errors}") + private int maxJsErrors; + + @Test + void givenTooBigScriptForEval_thenReturnError() { + String hugeScript = "var a = 'qwertyqwertywertyqwabababer'; return {a: a};"; + + assertThatThrownBy(() -> { + evalScript(hugeScript); + }).hasMessageContaining("body exceeds maximum allowed size"); + } + + @Test + void givenTooBigScriptForEval_whenMaxScriptBodySizeSetToZero_thenDoNothing() { + String script = "var a = 'a'; return { a: a };"; + + assertDoesNotThrow(() -> { + evalScript(script); + }); + } + + @Test + void givenTooBigScriptInputArgs_thenReturnErrorAndReportScriptExecutionError() throws Exception { + String script = "return { msg: msg };"; + String hugeMsg = "{\"input\":\"123456781234349\"}"; + UUID scriptId = evalScript(script); + + for (int i = 0; i < maxJsErrors; i++) { + assertThatThrownBy(() -> { + invokeScript(scriptId, hugeMsg); + }).hasMessageContaining("input arguments exceed maximum"); + } + assertThatScriptIsBlocked(scriptId); + } + + @Test + void whenScriptInvocationResultIsTooBig_thenReturnErrorAndReportScriptExecutionError() throws Exception { + String script = "var s = new Array(50).join('a'); return { s: s};"; + UUID scriptId = evalScript(script); + + for (int i = 0; i < maxJsErrors; i++) { + assertThatThrownBy(() -> { + invokeScript(scriptId, "{}"); + }).hasMessageContaining("result exceeds maximum allowed size"); + } + assertThatScriptIsBlocked(scriptId); + } + + private void assertThatScriptIsBlocked(UUID scriptId) { + assertThatThrownBy(() -> { + invokeScript(scriptId, "{}"); + }).hasMessageContaining("invocation is blocked due to maximum error"); + } + + private UUID evalScript(String script) throws ExecutionException, InterruptedException { + return jsInvokeService.eval(TenantId.SYS_TENANT_ID, JsScriptType.RULE_NODE_SCRIPT, script).get(); + } + + private String invokeScript(UUID scriptId, String msg) throws ExecutionException, InterruptedException { + return jsInvokeService.invokeFunction(TenantId.SYS_TENANT_ID, null, scriptId, msg, "{}", "POST_TELEMETRY_REQUEST").get(); + } + +} diff --git a/application/src/test/java/org/thingsboard/server/service/script/MockJsInvokeService.java b/application/src/test/java/org/thingsboard/server/service/script/MockJsInvokeService.java index 17803bac96..1d91f50131 100644 --- a/application/src/test/java/org/thingsboard/server/service/script/MockJsInvokeService.java +++ b/application/src/test/java/org/thingsboard/server/service/script/MockJsInvokeService.java @@ -37,7 +37,7 @@ public class MockJsInvokeService implements JsInvokeService { } @Override - public ListenableFuture invokeFunction(TenantId tenantId, CustomerId customerId, UUID scriptId, Object... args) { + public ListenableFuture invokeFunction(TenantId tenantId, CustomerId customerId, UUID scriptId, Object... args) { log.warn("invokeFunction {} {} {} {}", tenantId, customerId, scriptId, args); return Futures.immediateFuture("{}"); }