Skip to content

Commit 134613e

Browse files
committed
libexpr: Do not overflow heap buffer when there are too many formal arguments
3a3c062 introduced a buffer overflow for the case when there are more than 65535 formal arguments. It is a perfectly reasonable limitation, but we *must* not crash, corrupt memory or otherwise crash the process. Add a test for the graceful behavior and switch to using an explicit uninitialized_copy_n to further guard against buffer overflows.
1 parent 9d1907f commit 134613e

File tree

3 files changed

+40
-8
lines changed

3 files changed

+40
-8
lines changed

src/libexpr-tests/trivial.cc

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#include "nix/expr/tests/libexpr.hh"
2+
#include "nix/util/tests/gmock-matchers.hh"
23

34
namespace nix {
45
// Testing of trivial expressions
@@ -160,7 +161,8 @@ TEST_F(TrivialExpressionTest, assertPassed)
160161
ASSERT_THAT(v, IsIntEq(123));
161162
}
162163

163-
class AttrSetMergeTrvialExpressionTest : public TrivialExpressionTest, public testing::WithParamInterface<const char *>
164+
class AttrSetMergeTrvialExpressionTest : public TrivialExpressionTest,
165+
public ::testing::WithParamInterface<const char *>
164166
{};
165167

166168
TEST_P(AttrSetMergeTrvialExpressionTest, attrsetMergeLazy)
@@ -196,7 +198,7 @@ TEST_P(AttrSetMergeTrvialExpressionTest, attrsetMergeLazy)
196198
INSTANTIATE_TEST_SUITE_P(
197199
attrsetMergeLazy,
198200
AttrSetMergeTrvialExpressionTest,
199-
testing::Values("{ a.b = 1; a.c = 2; }", "{ a = { b = 1; }; a = { c = 2; }; }"));
201+
::testing::Values("{ a.b = 1; a.c = 2; }", "{ a = { b = 1; }; a = { c = 2; }; }"));
200202

201203
// The following macros ultimately define 48 tests (16 variations on three
202204
// templates). Each template tests an expression that can be written in 2^4
@@ -339,4 +341,18 @@ TEST_F(TrivialExpressionTest, orCantBeUsed)
339341
{
340342
ASSERT_THROW(eval("let or = 1; in or"), Error);
341343
}
344+
345+
TEST_F(TrivialExpressionTest, tooManyFormals)
346+
{
347+
std::string expr = "let f = { ";
348+
for (uint32_t i = 0; i <= std::numeric_limits<uint16_t>::max(); ++i) {
349+
expr += fmt("arg%d, ", i);
350+
}
351+
expr += " }: 0 in; f {}";
352+
ASSERT_THAT(
353+
[&]() { eval(expr); },
354+
::testing::ThrowsMessage<Error>(::nix::testing::HasSubstrIgnoreANSIMatcher(
355+
"too many formal arguments, implementation supports at most 65535")));
356+
}
357+
342358
} /* namespace nix */

src/libexpr/include/nix/expr/nixexpr.hh

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
#include "nix/expr/eval-error.hh"
1414
#include "nix/util/pos-idx.hh"
1515
#include "nix/expr/counter.hh"
16+
#include "nix/util/pos-table.hh"
17+
#include "nix/util/error.hh"
1618

1719
namespace nix {
1820

@@ -535,6 +537,7 @@ public:
535537
DocComment docComment;
536538

537539
ExprLambda(
540+
const PosTable & positions,
538541
std::pmr::polymorphic_allocator<char> & alloc,
539542
PosIdx pos,
540543
Symbol arg,
@@ -548,7 +551,15 @@ public:
548551
, formalsStart(alloc.allocate_object<Formal>(nFormals))
549552
, body(body)
550553
{
551-
std::ranges::copy(formals.formals, formalsStart);
554+
if (formals.formals.size() > nFormals) [[unlikely]] {
555+
auto err = Error(
556+
"too many formal arguments, implementation supports at most %1%",
557+
std::numeric_limits<decltype(nFormals)>::max());
558+
if (pos)
559+
err.atPos(positions[pos]);
560+
throw err;
561+
}
562+
std::uninitialized_copy_n(formals.formals.begin(), nFormals, formalsStart);
552563
};
553564

554565
ExprLambda(PosIdx pos, Symbol arg, Expr * body)
@@ -560,8 +571,13 @@ public:
560571
, formalsStart(nullptr)
561572
, body(body) {};
562573

563-
ExprLambda(std::pmr::polymorphic_allocator<char> & alloc, PosIdx pos, FormalsBuilder formals, Expr * body)
564-
: ExprLambda(alloc, pos, Symbol(), formals, body) {};
574+
ExprLambda(
575+
const PosTable & positions,
576+
std::pmr::polymorphic_allocator<char> & alloc,
577+
PosIdx pos,
578+
FormalsBuilder formals,
579+
Expr * body)
580+
: ExprLambda(positions, alloc, pos, Symbol(), formals, body) {};
565581

566582
void setName(Symbol name) override;
567583
std::string showNamePos(const EvalState & state) const;

src/libexpr/parser.y

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -186,23 +186,23 @@ expr_function
186186
| formal_set ':' expr_function[body]
187187
{
188188
state->validateFormals($formal_set);
189-
auto me = new ExprLambda(state->alloc, CUR_POS, std::move($formal_set), $body);
189+
auto me = new ExprLambda(state->positions, state->alloc, CUR_POS, std::move($formal_set), $body);
190190
$$ = me;
191191
SET_DOC_POS(me, @1);
192192
}
193193
| formal_set '@' ID ':' expr_function[body]
194194
{
195195
auto arg = state->symbols.create($ID);
196196
state->validateFormals($formal_set, CUR_POS, arg);
197-
auto me = new ExprLambda(state->alloc, CUR_POS, arg, std::move($formal_set), $body);
197+
auto me = new ExprLambda(state->positions, state->alloc, CUR_POS, arg, std::move($formal_set), $body);
198198
$$ = me;
199199
SET_DOC_POS(me, @1);
200200
}
201201
| ID '@' formal_set ':' expr_function[body]
202202
{
203203
auto arg = state->symbols.create($ID);
204204
state->validateFormals($formal_set, CUR_POS, arg);
205-
auto me = new ExprLambda(state->alloc, CUR_POS, arg, std::move($formal_set), $body);
205+
auto me = new ExprLambda(state->positions, state->alloc, CUR_POS, arg, std::move($formal_set), $body);
206206
$$ = me;
207207
SET_DOC_POS(me, @1);
208208
}

0 commit comments

Comments
 (0)