From 56d37656ac35a7c6be772562796fd4614d4e6a3a Mon Sep 17 00:00:00 2001 From: Philipp Otterbein Date: Sun, 13 Apr 2025 04:36:09 +0200 Subject: [PATCH] libexpr: fix UB in builtins.ceil and builtins.floor tighten and fix specification of both builtins --- src/libexpr-tests/primops.cc | 20 +++++++++++ src/libexpr/primops.cc | 68 +++++++++++++++++++++++++++++------- 2 files changed, 76 insertions(+), 12 deletions(-) diff --git a/src/libexpr-tests/primops.cc b/src/libexpr-tests/primops.cc index c3e50863d..7695a587a 100644 --- a/src/libexpr-tests/primops.cc +++ b/src/libexpr-tests/primops.cc @@ -56,11 +56,31 @@ namespace nix { TEST_F(PrimOpTest, ceil) { auto v = eval("builtins.ceil 1.9"); ASSERT_THAT(v, IsIntEq(2)); + auto intMin = eval("builtins.ceil (-4611686018427387904 - 4611686018427387904)"); + ASSERT_THAT(intMin, IsIntEq(std::numeric_limits::min())); + ASSERT_THROW(eval("builtins.ceil 1.0e200"), EvalError); + ASSERT_THROW(eval("builtins.ceil -1.0e200"), EvalError); + ASSERT_THROW(eval("builtins.ceil (1.0e200 * 1.0e200)"), EvalError); // inf + ASSERT_THROW(eval("builtins.ceil (-1.0e200 * 1.0e200)"), EvalError); // -inf + ASSERT_THROW(eval("builtins.ceil (1.0e200 * 1.0e200 - 1.0e200 * 1.0e200)"), EvalError); // nan + // bugs in previous Nix versions + ASSERT_THROW(eval("builtins.ceil (4611686018427387904 + 4611686018427387903)"), EvalError); + ASSERT_THROW(eval("builtins.ceil (-4611686018427387904 - 4611686018427387903)"), EvalError); } TEST_F(PrimOpTest, floor) { auto v = eval("builtins.floor 1.9"); ASSERT_THAT(v, IsIntEq(1)); + auto intMin = eval("builtins.ceil (-4611686018427387904 - 4611686018427387904)"); + ASSERT_THAT(intMin, IsIntEq(std::numeric_limits::min())); + ASSERT_THROW(eval("builtins.ceil 1.0e200"), EvalError); + ASSERT_THROW(eval("builtins.ceil -1.0e200"), EvalError); + ASSERT_THROW(eval("builtins.ceil (1.0e200 * 1.0e200)"), EvalError); // inf + ASSERT_THROW(eval("builtins.ceil (-1.0e200 * 1.0e200)"), EvalError); // -inf + ASSERT_THROW(eval("builtins.ceil (1.0e200 * 1.0e200 - 1.0e200 * 1.0e200)"), EvalError); // nan + // bugs in previous Nix versions + ASSERT_THROW(eval("builtins.ceil (4611686018427387904 + 4611686018427387903)"), EvalError); + ASSERT_THROW(eval("builtins.ceil (-4611686018427387904 - 4611686018427387903)"), EvalError); } TEST_F(PrimOpTest, tryEvalFailure) { diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 5e331f84d..5b24849d2 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -888,18 +888,40 @@ static void prim_ceil(EvalState & state, const PosIdx pos, Value * * args, Value { auto value = state.forceFloat(*args[0], args[0]->determinePos(pos), "while evaluating the first argument passed to builtins.ceil"); - v.mkInt(ceil(value)); + auto ceilValue = ceil(value); + bool isInt = args[0]->type() == nInt; + constexpr NixFloat int_min = std::numeric_limits::min(); // power of 2, so that no rounding occurs + if (ceilValue >= int_min && ceilValue < -int_min) { + v.mkInt(ceilValue); + } else if (isInt) { + // a NixInt, e.g. INT64_MAX, can be rounded to -int_min due to the cast to NixFloat + state.error("Due to a bug (see https://github.com/NixOS/nix/issues/12899) the NixInt argument %1% caused undefined behavior in previous Nix versions.\n\tFuture Nix versions might implement the correct behavior.", args[0]->integer().value).atPos(pos).debugThrow(); + } else { + state.error("NixFloat argument %1% is not in the range of NixInt", args[0]->fpoint()).atPos(pos).debugThrow(); + } + // `forceFloat` casts NixInt to NixFloat, but instead NixInt args shall be returned unmodified + if (isInt) { + auto arg = args[0]->integer(); + auto res = v.integer(); + if (arg != res) { + state.error("Due to a bug (see https://github.com/NixOS/nix/issues/12899) a loss of precision occured in previous Nix versions because the NixInt argument %1% was rounded to %2%.\n\tFuture Nix versions might implement the correct behavior.", arg, res).atPos(pos).debugThrow(); + } + } } static RegisterPrimOp primop_ceil({ .name = "__ceil", - .args = {"double"}, + .args = {"number"}, .doc = R"( - Converts an IEEE-754 double-precision floating-point number (*double*) to - the next higher integer. + Rounds and converts *number* to the next higher NixInt value if possible, i.e. `ceil *number* >= *number*` and + `ceil *number* - *number* < 1`. - If the datatype is neither an integer nor a "float", an evaluation error will be - thrown. + An evaluation error is thrown, if there exists no such NixInt value `ceil *number*`. + Due to bugs in previous Nix versions an evaluation error might be thrown, if the datatype of *number* is + a NixInt and if `*number* < -9007199254740992` or `*number* > 9007199254740992`. + + If the datatype of *number* is neither a NixInt (signed 64-bit integer) nor a NixFloat + (IEEE-754 double-precision floating-point number), an evaluation error will be thrown. )", .fun = prim_ceil, }); @@ -907,18 +929,40 @@ static RegisterPrimOp primop_ceil({ static void prim_floor(EvalState & state, const PosIdx pos, Value * * args, Value & v) { auto value = state.forceFloat(*args[0], args[0]->determinePos(pos), "while evaluating the first argument passed to builtins.floor"); - v.mkInt(floor(value)); + auto floorValue = floor(value); + bool isInt = args[0]->type() == nInt; + constexpr NixFloat int_min = std::numeric_limits::min(); // power of 2, so that no rounding occurs + if (floorValue >= int_min && floorValue < -int_min) { + v.mkInt(floorValue); + } else if (isInt) { + // a NixInt, e.g. INT64_MAX, can be rounded to -int_min due to the cast to NixFloat + state.error("Due to a bug (see https://github.com/NixOS/nix/issues/12899) the NixInt argument %1% caused undefined behavior in previous Nix versions.\n\tFuture Nix versions might implement the correct behavior.", args[0]->integer().value).atPos(pos).debugThrow(); + } else { + state.error("NixFloat argument %1% is not in the range of NixInt", args[0]->fpoint()).atPos(pos).debugThrow(); + } + // `forceFloat` casts NixInt to NixFloat, but instead NixInt args shall be returned unmodified + if (isInt) { + auto arg = args[0]->integer(); + auto res = v.integer(); + if (arg != res) { + state.error("Due to a bug (see https://github.com/NixOS/nix/issues/12899) a loss of precision occured in previous Nix versions because the NixInt argument %1% was rounded to %2%.\n\tFuture Nix versions might implement the correct behavior.", arg, res).atPos(pos).debugThrow(); + } + } } static RegisterPrimOp primop_floor({ .name = "__floor", - .args = {"double"}, + .args = {"number"}, .doc = R"( - Converts an IEEE-754 double-precision floating-point number (*double*) to - the next lower integer. + Rounds and converts *number* to the next lower NixInt value if possible, i.e. `floor *number* <= *number*` and + `*number* - floor *number* < 1`. - If the datatype is neither an integer nor a "float", an evaluation error will be - thrown. + An evaluation error is thrown, if there exists no such NixInt value `floor *number*`. + Due to bugs in previous Nix versions an evaluation error might be thrown, if the datatype of *number* is + a NixInt and if `*number* < -9007199254740992` or `*number* > 9007199254740992`. + + If the datatype of *number* is neither a NixInt (signed 64-bit integer) nor a NixFloat + (IEEE-754 double-precision floating-point number), an evaluation error will be thrown. )", .fun = prim_floor, });