From 33bd1141b4135d2915f981cbd1614687ac1a5b0b Mon Sep 17 00:00:00 2001 From: brharrington Date: Fri, 10 Nov 2023 07:44:34 -0600 Subject: [PATCH] core: add check for infinite loops (#1588) If the `:call` operator is used recursively it can result in an infinite loop. This change adds some sanity checks for the call depth to quickly fail in these cases and indicate that a loop was detected. --- .../atlas/core/stacklang/Context.scala | 15 ++++++- .../atlas/core/stacklang/Interpreter.scala | 9 +++- .../atlas/core/stacklang/LoopSuite.scala | 44 +++++++++++++++++++ 3 files changed, 66 insertions(+), 2 deletions(-) create mode 100644 atlas-core/src/test/scala/com/netflix/atlas/core/stacklang/LoopSuite.scala diff --git a/atlas-core/src/main/scala/com/netflix/atlas/core/stacklang/Context.scala b/atlas-core/src/main/scala/com/netflix/atlas/core/stacklang/Context.scala index bd3528331..7794f28b9 100644 --- a/atlas-core/src/main/scala/com/netflix/atlas/core/stacklang/Context.scala +++ b/atlas-core/src/main/scala/com/netflix/atlas/core/stacklang/Context.scala @@ -43,9 +43,12 @@ case class Context( variables: Map[String, Any], initialVariables: Map[String, Any] = Map.empty, frozenStack: List[Any] = Nil, - features: Features = Features.STABLE + features: Features = Features.STABLE, + callDepth: Int = 0 ) { + require(callDepth >= 0, "call depth cannot be negative") + /** * Remove the contents of the stack and push them onto the frozen stack. The variable * state will also be cleared. @@ -61,4 +64,14 @@ case class Context( def unfreeze: Context = { copy(stack = stack ::: frozenStack, frozenStack = Nil) } + + /** Increase the call depth for detecting deeply nested calls. */ + def incrementCallDepth: Context = { + copy(callDepth = callDepth + 1) + } + + /** Decrease the call depth for detecting deeply nested calls. */ + def decrementCallDepth: Context = { + copy(callDepth = callDepth - 1) + } } diff --git a/atlas-core/src/main/scala/com/netflix/atlas/core/stacklang/Interpreter.scala b/atlas-core/src/main/scala/com/netflix/atlas/core/stacklang/Interpreter.scala index 849b844eb..cc0f2fc00 100644 --- a/atlas-core/src/main/scala/com/netflix/atlas/core/stacklang/Interpreter.scala +++ b/atlas-core/src/main/scala/com/netflix/atlas/core/stacklang/Interpreter.scala @@ -103,11 +103,18 @@ case class Interpreter(vocabulary: List[Word]) { @scala.annotation.tailrec private def execute(s: Step): Context = { + if (s.context.callDepth > 10) { + // Prevent infinite loops. Operations like `:each` and `:map` to traverse a list are + // finite and will increase the depth by 1. The max call depth of 10 is arbitrary, but + // should be more than enough for legitimate use-cases. Testing 1M actual expressions, 3 + // was the highest depth seen. + throw new IllegalStateException(s"looping detected") + } if (s.program.isEmpty) s.context else execute(nextStep(s)) } final def execute(program: List[Any], context: Context, unfreeze: Boolean = true): Context = { - val result = execute(Step(program, context)) + val result = execute(Step(program, context.incrementCallDepth)).decrementCallDepth if (unfreeze) result.unfreeze else result } diff --git a/atlas-core/src/test/scala/com/netflix/atlas/core/stacklang/LoopSuite.scala b/atlas-core/src/test/scala/com/netflix/atlas/core/stacklang/LoopSuite.scala new file mode 100644 index 000000000..e47f587af --- /dev/null +++ b/atlas-core/src/test/scala/com/netflix/atlas/core/stacklang/LoopSuite.scala @@ -0,0 +1,44 @@ +/* + * Copyright 2014-2023 Netflix, Inc. + * + * 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 com.netflix.atlas.core.stacklang + +import munit.FunSuite + +class LoopSuite extends FunSuite { + + private val interpreter = Interpreter(StandardVocabulary.allWords) + + test("infinite loop: fcall recursion") { + val e = intercept[IllegalStateException] { + interpreter.execute("loop,(,loop,:fcall,),:set,loop,:fcall") + } + assertEquals(e.getMessage, "looping detected") + } + + test("infinite loop: call recursion") { + val e = intercept[IllegalStateException] { + interpreter.execute("loop,(,loop,:get,:call,),:set,loop,:get,:call") + } + assertEquals(e.getMessage, "looping detected") + } + + test("infinite loop: nested") { + val e = intercept[IllegalStateException] { + interpreter.execute("a,(,b,:fcall,),:set,b,(,c,:fcall,),:set,c,(,a,:fcall,),:set,a,:fcall") + } + assertEquals(e.getMessage, "looping detected") + } +}