Merge pull request #11188 from lf-/jade/kill-int-overflow

Ban integer overflow in the Nix language

tests/functional/lang/eval-fail-fetchTree-negative.err.exp

@@ -0,0 +1,8 @@
+error:
+       … while calling the 'fetchTree' builtin
+         at /pwd/lang/eval-fail-fetchTree-negative.nix:1:1:
+            1| builtins.fetchTree {
+             | ^
+            2|   type = "file";
+
+       error: negative value given for fetchTree attr owner: -1

tests/functional/lang/eval-fail-fetchTree-negative.nix

@@ -0,0 +1,5 @@
+builtins.fetchTree {
+  type = "file";
+  url = "file://eval-fail-fetchTree-negative.nix";
+  owner = -1;
+}

tests/functional/lang/eval-fail-flake-ref-to-string-negative-integer.err.exp

@@ -0,0 +1,14 @@
+error:
+       … while calling the 'seq' builtin
+         at /pwd/lang/eval-fail-flake-ref-to-string-negative-integer.nix:1:16:
+            1| let n = -1; in builtins.seq n (builtins.flakeRefToString {
+             |                ^
+            2|   type  = "github";
+
+       … while calling the 'flakeRefToString' builtin
+         at /pwd/lang/eval-fail-flake-ref-to-string-negative-integer.nix:1:32:
+            1| let n = -1; in builtins.seq n (builtins.flakeRefToString {
+             |                                ^
+            2|   type  = "github";
+
+       error: negative value given for flake ref attr repo: -1

tests/functional/lang/eval-fail-flake-ref-to-string-negative-integer.nix

@@ -0,0 +1,7 @@
+let n = -1; in builtins.seq n (builtins.flakeRefToString {
+  type  = "github";
+  owner = "NixOS";
+  repo  = n;
+  ref   = "23.05";
+  dir   = "lib";
+})

tests/functional/lang/eval-fail-fromJSON-overflowing.err.exp

@@ -0,0 +1,8 @@
+error:
+       … while calling the 'fromJSON' builtin
+         at /pwd/lang/eval-fail-fromJSON-overflowing.nix:1:1:
+            1| builtins.fromJSON ''{"attr": 18446744073709551615}''
+             | ^
+            2|
+
+       error: unsigned json number 18446744073709551615 outside of Nix integer range

tests/functional/lang/eval-fail-fromJSON-overflowing.nix

@@ -0,0 +1 @@
+builtins.fromJSON ''{"attr": 18446744073709551615}''

tests/functional/lang/eval-fail-overflowing-add.err.exp

@@ -0,0 +1,6 @@
+error: integer overflow in adding 9223372036854775807 + 1
+       at /pwd/lang/eval-fail-overflowing-add.nix:4:8:
+            3|   b = 1;
+            4| in a + b
+             |        ^
+            5|

tests/functional/lang/eval-fail-overflowing-add.nix

@@ -0,0 +1,4 @@
+let
+  a = 9223372036854775807;
+  b = 1;
+in a + b

tests/functional/lang/eval-fail-overflowing-div.err.exp

@@ -0,0 +1,23 @@
+error:
+       … while calling the 'seq' builtin
+         at /pwd/lang/eval-fail-overflowing-div.nix:7:4:
+            6|   b = -1;
+            7| in builtins.seq intMin (builtins.seq b (intMin / b))
+             |    ^
+            8|
+
+       … while calling the 'seq' builtin
+         at /pwd/lang/eval-fail-overflowing-div.nix:7:25:
+            6|   b = -1;
+            7| in builtins.seq intMin (builtins.seq b (intMin / b))
+             |                         ^
+            8|
+
+       … while calling the 'div' builtin
+         at /pwd/lang/eval-fail-overflowing-div.nix:7:48:
+            6|   b = -1;
+            7| in builtins.seq intMin (builtins.seq b (intMin / b))
+             |                                                ^
+            8|
+
+       error: integer overflow in dividing -9223372036854775808 / -1

tests/functional/lang/eval-fail-overflowing-div.nix

@@ -0,0 +1,7 @@
+let
+  # lol, this has to be written as an expression like this because negative
+  # numbers use unary negation rather than parsing directly, and 2**63 is out
+  # of range
+  intMin = -9223372036854775807 - 1;
+  b = -1;
+in builtins.seq intMin (builtins.seq b (intMin / b))

tests/functional/lang/eval-fail-overflowing-mul.err.exp

@@ -0,0 +1,16 @@
+error:
+       … while calling the 'mul' builtin
+         at /pwd/lang/eval-fail-overflowing-mul.nix:3:10:
+            2|   a = 4294967297;
+            3| in a * a * a
+             |          ^
+            4|
+
+       … while calling the 'mul' builtin
+         at /pwd/lang/eval-fail-overflowing-mul.nix:3:6:
+            2|   a = 4294967297;
+            3| in a * a * a
+             |      ^
+            4|
+
+       error: integer overflow in multiplying 4294967297 * 4294967297

tests/functional/lang/eval-fail-overflowing-mul.nix

@@ -0,0 +1,3 @@
+let
+  a = 4294967297;
+in a * a * a

tests/functional/lang/eval-fail-overflowing-sub.err.exp

@@ -0,0 +1,9 @@
+error:
+       … while calling the 'sub' builtin
+         at /pwd/lang/eval-fail-overflowing-sub.nix:4:6:
+            3|   b = 2;
+            4| in a - b
+             |      ^
+            5|
+
+       error: integer overflow in subtracting -9223372036854775807 - 2

tests/functional/lang/eval-fail-overflowing-sub.nix

@@ -0,0 +1,4 @@
+let
+  a = -9223372036854775807;
+  b = 2;
+in a - b

tests/unit/libexpr-support/tests/libexpr.hh

@@ -85,7 +85,7 @@ namespace nix {
         if (arg.type() != nInt) {
             return false;
         }
-        return arg.integer() == v;
+        return arg.integer().value == v;
     }
 
     MATCHER_P(IsFloatEq, v, fmt("The float is equal to \"%1%\"", v)) {

tests/unit/libutil-support/tests/gtest-with-params.hh

@@ -0,0 +1,54 @@
+#pragma once
+// SPDX-FileCopyrightText: 2014 Emil Eriksson
+//
+// SPDX-License-Identifier: BSD-2-Clause
+//
+// The lion's share of this code is copy pasted directly out of RapidCheck
+// headers, so the copyright is set accordingly.
+/**
+ * @file Implements the ability to run a RapidCheck test under gtest with changed
+ * test parameters such as the number of tests to run. This is useful for
+ * running very large numbers of the extremely cheap property tests.
+ */
+
+#include <gtest/gtest.h>
+#include <rapidcheck/gtest.h>
+#include <rapidcheck/gen/Arbitrary.hpp>
+
+namespace rc::detail {
+
+using MakeTestParams = TestParams (*)();
+
+template<typename Testable>
+void checkGTestWith(Testable && testable, MakeTestParams makeTestParams)
+{
+    const auto testInfo = ::testing::UnitTest::GetInstance()->current_test_info();
+    detail::TestMetadata metadata;
+    metadata.id = std::string(testInfo->test_case_name()) + "/" + std::string(testInfo->name());
+    metadata.description = std::string(testInfo->name());
+
+    const auto result = checkTestable(std::forward<Testable>(testable), metadata, makeTestParams());
+
+    if (result.template is<detail::SuccessResult>()) {
+        const auto success = result.template get<detail::SuccessResult>();
+        if (!success.distribution.empty()) {
+            printResultMessage(result, std::cout);
+            std::cout << std::endl;
+        }
+    } else {
+        std::ostringstream ss;
+        printResultMessage(result, ss);
+        FAIL() << ss.str() << std::endl;
+    }
+}
+}
+
+#define RC_GTEST_PROP_WITH_PARAMS(TestCase, Name, MakeParams, ArgList)                      \
+    void rapidCheck_propImpl_##TestCase##_##Name ArgList;                                   \
+                                                                                            \
+    TEST(TestCase, Name)                                                                    \
+    {                                                                                       \
+        ::rc::detail::checkGTestWith(&rapidCheck_propImpl_##TestCase##_##Name, MakeParams); \
+    }                                                                                       \
+                                                                                            \
+    void rapidCheck_propImpl_##TestCase##_##Name ArgList

tests/unit/libutil/checked-arithmetic.cc

@@ -0,0 +1,158 @@
+#include <cstdint>
+#include <gtest/gtest.h>
+#include <limits>
+#include <rapidcheck/Assertions.h>
+#include <rapidcheck/gtest.h>
+#include <rapidcheck/gen/Arbitrary.hpp>
+
+#include <checked-arithmetic.hh>
+
+#include "tests/gtest-with-params.hh"
+
+namespace rc {
+using namespace nix;
+
+template<std::integral T>
+struct Arbitrary<nix::checked::Checked<T>>
+{
+    static Gen<nix::checked::Checked<T>> arbitrary()
+    {
+        return gen::arbitrary<T>();
+    }
+};
+
+}
+
+namespace nix::checked {
+
+// Pointer to member function! Mildly gross.
+template<std::integral T>
+using Oper = Checked<T>::Result (Checked<T>::*)(T const other) const;
+
+template<std::integral T>
+using ReferenceOper = T (*)(T a, T b);
+
+/**
+ * Checks that performing an operation that overflows into an inaccurate result
+ * has the desired behaviour.
+ *
+ * TBig is a type large enough to represent all results of TSmall operations.
+ */
+template<std::integral TSmall, std::integral TBig>
+void checkType(TSmall a_, TSmall b, Oper<TSmall> oper, ReferenceOper<TBig> reference)
+{
+    // Sufficient to fit all values
+    TBig referenceResult = reference(a_, b);
+    constexpr const TSmall minV = std::numeric_limits<TSmall>::min();
+    constexpr const TSmall maxV = std::numeric_limits<TSmall>::max();
+
+    Checked<TSmall> a{a_};
+    auto result = (a.*(oper))(b);
+
+    // Just truncate it to get the in-range result
+    RC_ASSERT(result.valueWrapping() == static_cast<TSmall>(referenceResult));
+
+    if (referenceResult > maxV || referenceResult < minV) {
+        RC_ASSERT(result.overflowed());
+        RC_ASSERT(!result.valueChecked().has_value());
+    } else {
+        RC_ASSERT(!result.overflowed());
+        RC_ASSERT(result.valueChecked().has_value());
+        RC_ASSERT(*result.valueChecked() == referenceResult);
+    }
+}
+
+/**
+ * Checks that performing an operation that overflows into an inaccurate result
+ * has the desired behaviour.
+ *
+ * TBig is a type large enough to represent all results of TSmall operations.
+ */
+template<std::integral TSmall, std::integral TBig>
+void checkDivision(TSmall a_, TSmall b)
+{
+    // Sufficient to fit all values
+    constexpr const TSmall minV = std::numeric_limits<TSmall>::min();
+
+    Checked<TSmall> a{a_};
+    auto result = a / b;
+
+    if (std::is_signed<TSmall>() && a_ == minV && b == -1) {
+        // This is the only possible overflow condition
+        RC_ASSERT(result.valueWrapping() == minV);
+        RC_ASSERT(result.overflowed());
+    } else if (b == 0) {
+        RC_ASSERT(result.divideByZero());
+        RC_ASSERT_THROWS_AS(result.valueWrapping(), nix::checked::DivideByZero);
+        RC_ASSERT(result.valueChecked() == std::nullopt);
+    } else {
+        TBig referenceResult = a_ / b;
+        auto result_ = result.valueChecked();
+        RC_ASSERT(result_.has_value());
+        RC_ASSERT(*result_ == referenceResult);
+        RC_ASSERT(result.valueWrapping() == referenceResult);
+    }
+}
+
+/** Creates parameters that perform a more adequate number of checks to validate
+ * extremely cheap tests such as arithmetic tests */
+static rc::detail::TestParams makeParams()
+{
+    auto const & conf = rc::detail::configuration();
+    auto newParams = conf.testParams;
+    newParams.maxSuccess = 10000;
+    return newParams;
+}
+
+RC_GTEST_PROP_WITH_PARAMS(Checked, add_unsigned, makeParams, (uint16_t a, uint16_t b))
+{
+    checkType<uint16_t, int32_t>(a, b, &Checked<uint16_t>::operator+, [](int32_t a, int32_t b) { return a + b; });
+}
+
+RC_GTEST_PROP_WITH_PARAMS(Checked, add_signed, makeParams, (int16_t a, int16_t b))
+{
+    checkType<int16_t, int32_t>(a, b, &Checked<int16_t>::operator+, [](int32_t a, int32_t b) { return a + b; });
+}
+
+RC_GTEST_PROP_WITH_PARAMS(Checked, sub_unsigned, makeParams, (uint16_t a, uint16_t b))
+{
+    checkType<uint16_t, int32_t>(a, b, &Checked<uint16_t>::operator-, [](int32_t a, int32_t b) { return a - b; });
+}
+
+RC_GTEST_PROP_WITH_PARAMS(Checked, sub_signed, makeParams, (int16_t a, int16_t b))
+{
+    checkType<int16_t, int32_t>(a, b, &Checked<int16_t>::operator-, [](int32_t a, int32_t b) { return a - b; });
+}
+
+RC_GTEST_PROP_WITH_PARAMS(Checked, mul_unsigned, makeParams, (uint16_t a, uint16_t b))
+{
+    checkType<uint16_t, int64_t>(a, b, &Checked<uint16_t>::operator*, [](int64_t a, int64_t b) { return a * b; });
+}
+
+RC_GTEST_PROP_WITH_PARAMS(Checked, mul_signed, makeParams, (int16_t a, int16_t b))
+{
+    checkType<int16_t, int64_t>(a, b, &Checked<int16_t>::operator*, [](int64_t a, int64_t b) { return a * b; });
+}
+
+RC_GTEST_PROP_WITH_PARAMS(Checked, div_unsigned, makeParams, (uint16_t a, uint16_t b))
+{
+    checkDivision<uint16_t, int64_t>(a, b);
+}
+
+RC_GTEST_PROP_WITH_PARAMS(Checked, div_signed, makeParams, (int16_t a, int16_t b))
+{
+    checkDivision<int16_t, int64_t>(a, b);
+}
+
+// Make absolutely sure that we check the special cases if the proptest
+// generator does not come up with them. This one is especially important
+// because it has very specific pairs required for the edge cases unlike the
+// others.
+TEST(Checked, div_signed_special_cases)
+{
+    checkDivision<int16_t, int64_t>(std::numeric_limits<int16_t>::min(), -1);
+    checkDivision<int16_t, int64_t>(std::numeric_limits<int16_t>::min(), 0);
+    checkDivision<int16_t, int64_t>(0, 0);
+}
+
+}