Skip to content

Commit 3aee49a

Browse files
authored
Merge commit from fork
Backport for support/2.14
2 parents 4bacdfd + c508d97 commit 3aee49a

13 files changed

+651
-42
lines changed

lib/base/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ set(base_SOURCES
6262
ringbuffer.cpp ringbuffer.hpp
6363
scriptframe.cpp scriptframe.hpp
6464
scriptglobal.cpp scriptglobal.hpp
65+
scriptpermission.cpp scriptpermission.hpp
6566
scriptutils.cpp scriptutils.hpp
6667
serializer.cpp serializer.hpp
6768
shared.hpp

lib/base/scriptframe.cpp

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,24 @@ INITIALIZE_ONCE_WITH_PRIORITY([]() {
4444
l_StatsNS->Freeze();
4545
}, InitializePriority::FreezeNamespaces);
4646

47+
/**
48+
* Construct a @c ScriptFrame that has `Self` assigned to the global namespace.
49+
*
50+
* Prefer the other constructor if possible since if misused this may leak global variables
51+
* without permissions or senstive variables like TicketSalt in a sandboxed context.
52+
*
53+
* @todo Remove this constructor and call the other with the global namespace in places where it's actually necessary.
54+
*/
4755
ScriptFrame::ScriptFrame(bool allocLocals)
48-
: Locals(allocLocals ? new Dictionary() : nullptr), Self(ScriptGlobal::GetGlobals()), Sandboxed(false), Depth(0)
56+
: Locals(allocLocals ? new Dictionary() : nullptr), PermChecker(new ScriptPermissionChecker),
57+
Self(ScriptGlobal::GetGlobals()), Sandboxed(false), Depth(0), Globals(nullptr)
4958
{
5059
InitializeFrame();
5160
}
5261

5362
ScriptFrame::ScriptFrame(bool allocLocals, Value self)
54-
: Locals(allocLocals ? new Dictionary() : nullptr), Self(std::move(self)), Sandboxed(false), Depth(0)
63+
: Locals(allocLocals ? new Dictionary() : nullptr), PermChecker(new ScriptPermissionChecker), Self(std::move(self)),
64+
Sandboxed(false), Depth(0), Globals(nullptr)
5565
{
5666
InitializeFrame();
5767
}
@@ -63,6 +73,8 @@ void ScriptFrame::InitializeFrame()
6373
if (frames && !frames->empty()) {
6474
ScriptFrame *frame = frames->top();
6575

76+
// See the documentation of `ScriptFrame::Globals` for why these two are inherited and Globals isn't.
77+
PermChecker = frame->PermChecker;
6678
Sandboxed = frame->Sandboxed;
6779
}
6880

@@ -79,6 +91,41 @@ ScriptFrame::~ScriptFrame()
7991
#endif /* I2_DEBUG */
8092
}
8193

94+
/**
95+
* Returns a sanitized copy of the global variables namespace when sandboxed.
96+
*
97+
* This filters out the TicketSalt variable specifically and any variable for which the
98+
* PermChecker does not return 'true'.
99+
*
100+
* However it specifically keeps the Types, System, and Icinga sub-namespaces, because they're
101+
* accessed through globals in ScopeExpression and the user should have access to all Values
102+
* contained in these namespaces.
103+
*
104+
* @return a sanitized copy of the global namespace if sandboxed, a pointer to the global namespace otherwise.
105+
*/
106+
Namespace::Ptr ScriptFrame::GetGlobals()
107+
{
108+
if (Sandboxed) {
109+
if (!Globals) {
110+
Globals = new Namespace;
111+
auto globals = ScriptGlobal::GetGlobals();
112+
ObjectLock lock{globals};
113+
for (auto& [key, val] : globals) {
114+
if (key == "TicketSalt") {
115+
continue;
116+
}
117+
118+
if (key == "Types" || key == "System" || key == "Icinga" || PermChecker->CanAccessGlobalVariable(key)) {
119+
Globals->Set(key, val.Val, val.Const);
120+
}
121+
}
122+
}
123+
return Globals;
124+
}
125+
126+
return ScriptGlobal::GetGlobals();
127+
}
128+
82129
void ScriptFrame::IncreaseStackDepth()
83130
{
84131
if (Depth + 1 > 300)

lib/base/scriptframe.hpp

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,30 @@
55

66
#include "base/i2-base.hpp"
77
#include "base/dictionary.hpp"
8-
#include "base/array.hpp"
8+
#include "base/namespace.hpp"
9+
#include "base/scriptpermission.hpp"
910
#include <boost/thread/tss.hpp>
1011
#include <stack>
1112

1213
namespace icinga
1314
{
1415

16+
/**
17+
* A frame describing the context a section of script code is executed in.
18+
*
19+
* This is implemented by each new object that is constructed getting pushed on a thread_local
20+
* global stack that is accessible from anywhere during script evaluation.
21+
*
22+
* Most properties in this frame, like local variables do not carry over to successive frames,
23+
* except the `PermChecker` and `Sandboxed` members, which get propagated to enforce access
24+
* control and availability of unsafe functions.
25+
*/
1526
struct ScriptFrame
1627
{
1728
Dictionary::Ptr Locals;
29+
ScriptPermissionChecker::Ptr PermChecker; /* inherited by next frame */
1830
Value Self;
19-
bool Sandboxed;
31+
bool Sandboxed; /* inherited by next frame */
2032
int Depth;
2133

2234
ScriptFrame(bool allocLocals);
@@ -28,7 +40,20 @@ struct ScriptFrame
2840

2941
static ScriptFrame *GetCurrentFrame();
3042

43+
Namespace::Ptr GetGlobals();
44+
3145
private:
46+
/**
47+
* Caches a sanitized version of the global namespace for the current `ScriptFrame`.
48+
*
49+
* This is a value that is dependent on a ScriptFrame's `Sandboxed` and `CheckPerms`
50+
* members. These are both independent of each other and while they are inherited by
51+
* subsequent frames themselves, their values can be changed for new frames easily.
52+
* Therefore Globals can hold a different value for each ScriptFrame and is not
53+
* inherited.
54+
*/
55+
Namespace::Ptr Globals;
56+
3257
static boost::thread_specific_ptr<std::stack<ScriptFrame *> > m_ScriptFrames;
3358

3459
static void PushFrame(ScriptFrame *frame);

lib/base/scriptpermission.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/* Icinga 2 | (c) 2025 Icinga GmbH | GPLv2+ */
2+
3+
#include "base/scriptpermission.hpp"
4+
5+
using namespace icinga;
6+
7+
bool ScriptPermissionChecker::CanAccessGlobalVariable(const String&)
8+
{
9+
return true;
10+
}
11+
12+
bool ScriptPermissionChecker::CanAccessConfigObject(const ConfigObject::Ptr&)
13+
{
14+
return true;
15+
}

lib/base/scriptpermission.hpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/* Icinga 2 | (c) 2025 Icinga GmbH | GPLv2+ */
2+
3+
#pragma once
4+
5+
#include "base/string.hpp"
6+
#include "base/shared-object.hpp"
7+
#include "base/configobject.hpp"
8+
9+
namespace icinga {
10+
11+
class ScriptPermissionChecker : public SharedObject
12+
{
13+
public:
14+
DECLARE_PTR_TYPEDEFS(ScriptPermissionChecker);
15+
16+
ScriptPermissionChecker() = default;
17+
ScriptPermissionChecker(const ScriptPermissionChecker&) = delete;
18+
ScriptPermissionChecker(ScriptPermissionChecker&&) = delete;
19+
ScriptPermissionChecker& operator=(const ScriptPermissionChecker&) = delete;
20+
ScriptPermissionChecker& operator=(ScriptPermissionChecker&&) = delete;
21+
22+
~ScriptPermissionChecker() override = default;
23+
24+
virtual bool CanAccessGlobalVariable(const String& varName);
25+
virtual bool CanAccessConfigObject(const ConfigObject::Ptr& obj);
26+
};
27+
28+
} // namespace icinga

lib/base/scriptutils.cpp

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,18 +36,18 @@ REGISTER_FUNCTION(System, exit, &Application::Exit, "status");
3636
REGISTER_SAFE_FUNCTION(System, typeof, &ScriptUtils::TypeOf, "value");
3737
REGISTER_SAFE_FUNCTION(System, keys, &ScriptUtils::Keys, "value");
3838
REGISTER_SAFE_FUNCTION(System, random, &Utility::Random, "");
39-
REGISTER_SAFE_FUNCTION(System, get_template, &ScriptUtils::GetTemplate, "type:name");
40-
REGISTER_SAFE_FUNCTION(System, get_templates, &ScriptUtils::GetTemplates, "type");
39+
REGISTER_FUNCTION(System, get_template, &ScriptUtils::GetTemplate, "type:name");
40+
REGISTER_FUNCTION(System, get_templates, &ScriptUtils::GetTemplates, "type");
4141
REGISTER_SAFE_FUNCTION(System, get_object, &ScriptUtils::GetObject, "type:name");
42-
REGISTER_SAFE_FUNCTION(System, get_objects, &ScriptUtils::GetObjects, "type");
42+
REGISTER_FUNCTION(System, get_objects, &ScriptUtils::GetObjects, "type");
4343
REGISTER_FUNCTION(System, assert, &ScriptUtils::Assert, "value");
4444
REGISTER_SAFE_FUNCTION(System, string, &ScriptUtils::CastString, "value");
4545
REGISTER_SAFE_FUNCTION(System, number, &ScriptUtils::CastNumber, "value");
4646
REGISTER_SAFE_FUNCTION(System, bool, &ScriptUtils::CastBool, "value");
4747
REGISTER_SAFE_FUNCTION(System, get_time, &Utility::GetTime, "");
4848
REGISTER_SAFE_FUNCTION(System, basename, &Utility::BaseName, "path");
4949
REGISTER_SAFE_FUNCTION(System, dirname, &Utility::DirName, "path");
50-
REGISTER_SAFE_FUNCTION(System, getenv, &ScriptUtils::GetEnv, "value");
50+
REGISTER_FUNCTION(System, getenv, &ScriptUtils::GetEnv, "value");
5151
REGISTER_SAFE_FUNCTION(System, msi_get_component_path, &ScriptUtils::MsiGetComponentPathShim, "component");
5252
REGISTER_SAFE_FUNCTION(System, track_parents, &ScriptUtils::TrackParents, "child");
5353
REGISTER_SAFE_FUNCTION(System, escape_shell_cmd, &Utility::EscapeShellCmd, "cmd");
@@ -475,7 +475,15 @@ ConfigObject::Ptr ScriptUtils::GetObject(const Value& vtype, const String& name)
475475
if (!ctype)
476476
return nullptr;
477477

478-
return ctype->GetObject(name);
478+
auto cfgObj = ctype->GetObject(name);
479+
if (cfgObj) {
480+
auto* frame = ScriptFrame::GetCurrentFrame();
481+
if (frame->PermChecker->CanAccessConfigObject(cfgObj)) {
482+
return cfgObj;
483+
}
484+
}
485+
486+
return nullptr;
479487
}
480488

481489
Array::Ptr ScriptUtils::GetObjects(const Type::Ptr& type)

lib/config/expression.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,10 @@ ExpressionResult VariableExpression::DoEvaluate(ScriptFrame& frame, DebugHint *d
118118
return value;
119119
else if (VMOps::FindVarImport(frame, m_Imports, m_Variable, &value, m_DebugInfo))
120120
return value;
121-
else
122-
return ScriptGlobal::Get(m_Variable);
121+
else if (frame.GetGlobals()->Get(m_Variable, &value))
122+
return value;
123+
124+
BOOST_THROW_EXCEPTION(ScriptError{"Tried to access undefined script variable '" + m_Variable + "'"});
123125
}
124126

125127
bool VariableExpression::GetReference(ScriptFrame& frame, bool init_dict, Value *parent, String *index, DebugHint **dhint) const
@@ -138,8 +140,8 @@ bool VariableExpression::GetReference(ScriptFrame& frame, bool init_dict, Value
138140
*dhint = new DebugHint((*dhint)->GetChild(m_Variable));
139141
} else if (VMOps::FindVarImportRef(frame, m_Imports, m_Variable, parent, m_DebugInfo)) {
140142
return true;
141-
} else if (ScriptGlobal::Exists(m_Variable)) {
142-
*parent = ScriptGlobal::GetGlobals();
143+
} else if (frame.GetGlobals()->Contains(m_Variable)) {
144+
*parent = frame.GetGlobals();
143145

144146
if (dhint)
145147
*dhint = nullptr;
@@ -546,7 +548,7 @@ ExpressionResult GetScopeExpression::DoEvaluate(ScriptFrame& frame, DebugHint *d
546548
else if (m_ScopeSpec == ScopeThis)
547549
return frame.Self;
548550
else if (m_ScopeSpec == ScopeGlobal)
549-
return ScriptGlobal::GetGlobals();
551+
return frame.GetGlobals();
550552
else
551553
VERIFY(!"Invalid scope.");
552554
}

0 commit comments

Comments
 (0)