From 7da40832376aa66f270f4398bb1b77c471f8d225 Mon Sep 17 00:00:00 2001
From: Azat Khuzhin <a3at.mail@gmail.com>
Date: Sat, 30 Jan 2021 07:13:49 +0300
Subject: [PATCH] Forbid bit functions for floats

This will fix the following UB report [1]:

  "../src/Functions/bitAnd.cpp:24:61: runtime error: nan is outside the
range of representable values of type 'long' Received signal -3 Received
signal Unknown signal (-3)"

  [1]: https://clickhouse-test-reports.s3.yandex.net/19824/89c4055202b9d08459f90ee5791d4e3017b82fbf/fuzzer_ubsan/report.html#fail1
---
 src/Functions/FunctionBinaryArithmetic.h      | 62 +++++++++++++++----
 src/Functions/bitAnd.cpp                      |  2 +-
 src/Functions/bitOr.cpp                       |  2 +-
 src/Functions/bitRotateLeft.cpp               |  2 +-
 src/Functions/bitRotateRight.cpp              |  2 +-
 src/Functions/bitShiftLeft.cpp                |  2 +-
 src/Functions/bitShiftRight.cpp               |  2 +-
 src/Functions/bitTest.cpp                     |  2 +-
 src/Functions/bitXor.cpp                      |  2 +-
 .../01576_if_null_external_aggregation.sql    |  2 +-
 .../0_stateless/01677_bit_float.reference     |  0
 tests/queries/0_stateless/01677_bit_float.sql |  9 +++
 12 files changed, 68 insertions(+), 21 deletions(-)
 create mode 100644 tests/queries/0_stateless/01677_bit_float.reference
 create mode 100644 tests/queries/0_stateless/01677_bit_float.sql

diff --git a/src/Functions/FunctionBinaryArithmetic.h b/src/Functions/FunctionBinaryArithmetic.h
index c10ff581b0e..f61c9c91d00 100644
--- a/src/Functions/FunctionBinaryArithmetic.h
+++ b/src/Functions/FunctionBinaryArithmetic.h
@@ -504,7 +504,7 @@ private:
 using namespace traits_;
 using namespace impl_;
 
-template <template <typename, typename> class Op, typename Name, bool valid_on_default_arguments = true>
+template <template <typename, typename> class Op, typename Name, bool valid_on_default_arguments = true, bool valid_on_float_arguments = true>
 class FunctionBinaryArithmetic : public IFunction
 {
     static constexpr const bool is_plus = IsOperation<Op>::plus;
@@ -542,16 +542,54 @@ class FunctionBinaryArithmetic : public IFunction
         >(type, std::forward<F>(f));
     }
 
+    template <typename F>
+    static bool castTypeNoFloats(const IDataType * type, F && f)
+    {
+        return castTypeToEither<
+            DataTypeUInt8,
+            DataTypeUInt16,
+            DataTypeUInt32,
+            DataTypeUInt64,
+            DataTypeUInt256,
+            DataTypeInt8,
+            DataTypeInt16,
+            DataTypeInt32,
+            DataTypeInt64,
+            DataTypeInt128,
+            DataTypeInt256,
+            DataTypeDate,
+            DataTypeDateTime,
+            DataTypeDecimal<Decimal32>,
+            DataTypeDecimal<Decimal64>,
+            DataTypeDecimal<Decimal128>,
+            DataTypeDecimal<Decimal256>,
+            DataTypeFixedString
+        >(type, std::forward<F>(f));
+    }
+
     template <typename F>
     static bool castBothTypes(const IDataType * left, const IDataType * right, F && f)
     {
-        return castType(left, [&](const auto & left_)
+        if constexpr (valid_on_float_arguments)
         {
-            return castType(right, [&](const auto & right_)
+            return castType(left, [&](const auto & left_)
             {
-                return f(left_, right_);
+                return castType(right, [&](const auto & right_)
+                {
+                    return f(left_, right_);
+                });
             });
-        });
+        }
+        else
+        {
+            return castTypeNoFloats(left, [&](const auto & left_)
+            {
+                return castTypeNoFloats(right, [&](const auto & right_)
+                {
+                    return f(left_, right_);
+                });
+            });
+        }
     }
 
     static FunctionOverloadResolverPtr
@@ -1319,11 +1357,11 @@ public:
 };
 
 
-template <template <typename, typename> class Op, typename Name, bool valid_on_default_arguments = true>
-class FunctionBinaryArithmeticWithConstants : public FunctionBinaryArithmetic<Op, Name, valid_on_default_arguments>
+template <template <typename, typename> class Op, typename Name, bool valid_on_default_arguments = true, bool valid_on_float_arguments = true>
+class FunctionBinaryArithmeticWithConstants : public FunctionBinaryArithmetic<Op, Name, valid_on_default_arguments, valid_on_float_arguments>
 {
 public:
-    using Base = FunctionBinaryArithmetic<Op, Name, valid_on_default_arguments>;
+    using Base = FunctionBinaryArithmetic<Op, Name, valid_on_default_arguments, valid_on_float_arguments>;
     using Monotonicity = typename Base::Monotonicity;
 
     static FunctionPtr create(
@@ -1488,7 +1526,7 @@ private:
     DataTypePtr return_type;
 };
 
-template <template <typename, typename> class Op, typename Name, bool valid_on_default_arguments = true>
+template <template <typename, typename> class Op, typename Name, bool valid_on_default_arguments = true, bool valid_on_float_arguments = true>
 class BinaryArithmeticOverloadResolver : public IFunctionOverloadResolverImpl
 {
 public:
@@ -1512,14 +1550,14 @@ public:
                 || (arguments[1].column && isColumnConst(*arguments[1].column))))
         {
             return std::make_unique<DefaultFunction>(
-                FunctionBinaryArithmeticWithConstants<Op, Name, valid_on_default_arguments>::create(
+                FunctionBinaryArithmeticWithConstants<Op, Name, valid_on_default_arguments, valid_on_float_arguments>::create(
                     arguments[0], arguments[1], return_type, context),
                 ext::map<DataTypes>(arguments, [](const auto & elem) { return elem.type; }),
                 return_type);
         }
 
         return std::make_unique<DefaultFunction>(
-            FunctionBinaryArithmetic<Op, Name, valid_on_default_arguments>::create(context),
+            FunctionBinaryArithmetic<Op, Name, valid_on_default_arguments, valid_on_float_arguments>::create(context),
             ext::map<DataTypes>(arguments, [](const auto & elem) { return elem.type; }),
             return_type);
     }
@@ -1530,7 +1568,7 @@ public:
             throw Exception(
                 "Number of arguments for function " + getName() + " doesn't match: passed " + toString(arguments.size()) + ", should be 2",
                 ErrorCodes::NUMBER_OF_ARGUMENTS_DOESNT_MATCH);
-        return FunctionBinaryArithmetic<Op, Name, valid_on_default_arguments>::getReturnTypeImplStatic(arguments, context);
+        return FunctionBinaryArithmetic<Op, Name, valid_on_default_arguments, valid_on_float_arguments>::getReturnTypeImplStatic(arguments, context);
     }
 
 private:
diff --git a/src/Functions/bitAnd.cpp b/src/Functions/bitAnd.cpp
index 08fc4699c36..aa4b7593c79 100644
--- a/src/Functions/bitAnd.cpp
+++ b/src/Functions/bitAnd.cpp
@@ -37,7 +37,7 @@ struct BitAndImpl
 };
 
 struct NameBitAnd { static constexpr auto name = "bitAnd"; };
-using FunctionBitAnd = BinaryArithmeticOverloadResolver<BitAndImpl, NameBitAnd, true>;
+using FunctionBitAnd = BinaryArithmeticOverloadResolver<BitAndImpl, NameBitAnd, true, false>;
 
 }
 
diff --git a/src/Functions/bitOr.cpp b/src/Functions/bitOr.cpp
index 400c478fcbf..adf2244bc0a 100644
--- a/src/Functions/bitOr.cpp
+++ b/src/Functions/bitOr.cpp
@@ -36,7 +36,7 @@ struct BitOrImpl
 };
 
 struct NameBitOr { static constexpr auto name = "bitOr"; };
-using FunctionBitOr = BinaryArithmeticOverloadResolver<BitOrImpl, NameBitOr, true>;
+using FunctionBitOr = BinaryArithmeticOverloadResolver<BitOrImpl, NameBitOr, true, false>;
 
 }
 
diff --git a/src/Functions/bitRotateLeft.cpp b/src/Functions/bitRotateLeft.cpp
index aac0197f2c5..9962ac174fd 100644
--- a/src/Functions/bitRotateLeft.cpp
+++ b/src/Functions/bitRotateLeft.cpp
@@ -43,7 +43,7 @@ struct BitRotateLeftImpl
 };
 
 struct NameBitRotateLeft { static constexpr auto name = "bitRotateLeft"; };
-using FunctionBitRotateLeft = BinaryArithmeticOverloadResolver<BitRotateLeftImpl, NameBitRotateLeft>;
+using FunctionBitRotateLeft = BinaryArithmeticOverloadResolver<BitRotateLeftImpl, NameBitRotateLeft, true, false>;
 
 }
 
diff --git a/src/Functions/bitRotateRight.cpp b/src/Functions/bitRotateRight.cpp
index e8932eccaa3..8352f2f92bc 100644
--- a/src/Functions/bitRotateRight.cpp
+++ b/src/Functions/bitRotateRight.cpp
@@ -42,7 +42,7 @@ struct BitRotateRightImpl
 };
 
 struct NameBitRotateRight { static constexpr auto name = "bitRotateRight"; };
-using FunctionBitRotateRight = BinaryArithmeticOverloadResolver<BitRotateRightImpl, NameBitRotateRight>;
+using FunctionBitRotateRight = BinaryArithmeticOverloadResolver<BitRotateRightImpl, NameBitRotateRight, true, false>;
 
 }
 
diff --git a/src/Functions/bitShiftLeft.cpp b/src/Functions/bitShiftLeft.cpp
index 9f8adf39741..96e0fbe6d6f 100644
--- a/src/Functions/bitShiftLeft.cpp
+++ b/src/Functions/bitShiftLeft.cpp
@@ -42,7 +42,7 @@ struct BitShiftLeftImpl
 };
 
 struct NameBitShiftLeft { static constexpr auto name = "bitShiftLeft"; };
-using FunctionBitShiftLeft = BinaryArithmeticOverloadResolver<BitShiftLeftImpl, NameBitShiftLeft>;
+using FunctionBitShiftLeft = BinaryArithmeticOverloadResolver<BitShiftLeftImpl, NameBitShiftLeft, true, false>;
 
 }
 
diff --git a/src/Functions/bitShiftRight.cpp b/src/Functions/bitShiftRight.cpp
index bc54d3c8b5d..37c3908f8de 100644
--- a/src/Functions/bitShiftRight.cpp
+++ b/src/Functions/bitShiftRight.cpp
@@ -42,7 +42,7 @@ struct BitShiftRightImpl
 };
 
 struct NameBitShiftRight { static constexpr auto name = "bitShiftRight"; };
-using FunctionBitShiftRight = BinaryArithmeticOverloadResolver<BitShiftRightImpl, NameBitShiftRight>;
+using FunctionBitShiftRight = BinaryArithmeticOverloadResolver<BitShiftRightImpl, NameBitShiftRight, true, false>;
 
 }
 
diff --git a/src/Functions/bitTest.cpp b/src/Functions/bitTest.cpp
index 54c932d9311..b7e9a84c6c1 100644
--- a/src/Functions/bitTest.cpp
+++ b/src/Functions/bitTest.cpp
@@ -34,7 +34,7 @@ struct BitTestImpl
 };
 
 struct NameBitTest { static constexpr auto name = "bitTest"; };
-using FunctionBitTest = BinaryArithmeticOverloadResolver<BitTestImpl, NameBitTest>;
+using FunctionBitTest = BinaryArithmeticOverloadResolver<BitTestImpl, NameBitTest, true, false>;
 
 }
 
diff --git a/src/Functions/bitXor.cpp b/src/Functions/bitXor.cpp
index c5867c8d2ca..d8901c6c9ff 100644
--- a/src/Functions/bitXor.cpp
+++ b/src/Functions/bitXor.cpp
@@ -36,7 +36,7 @@ struct BitXorImpl
 };
 
 struct NameBitXor { static constexpr auto name = "bitXor"; };
-using FunctionBitXor = BinaryArithmeticOverloadResolver<BitXorImpl, NameBitXor, true>;
+using FunctionBitXor = BinaryArithmeticOverloadResolver<BitXorImpl, NameBitXor, true, false>;
 
 }
 
diff --git a/tests/queries/0_stateless/01576_if_null_external_aggregation.sql b/tests/queries/0_stateless/01576_if_null_external_aggregation.sql
index b9c36a9cecc..cffeb73b1ca 100644
--- a/tests/queries/0_stateless/01576_if_null_external_aggregation.sql
+++ b/tests/queries/0_stateless/01576_if_null_external_aggregation.sql
@@ -3,5 +3,5 @@ SET max_bytes_before_external_group_by = 200000000;
 SET max_memory_usage = 1500000000;
 SET max_threads = 12;
 
-SELECT bitAnd(number, pow(2, 20) - 1) as k, argMaxIf(k, number % 2 = 0 ? number : Null, number > 42),  uniq(number) AS u FROM numbers(1000000) GROUP BY k format Null;
+SELECT bitAnd(number, toUInt64(pow(2, 20) - 1)) as k, argMaxIf(k, number % 2 = 0 ? number : Null, number > 42),  uniq(number) AS u FROM numbers(1000000) GROUP BY k format Null;
 
diff --git a/tests/queries/0_stateless/01677_bit_float.reference b/tests/queries/0_stateless/01677_bit_float.reference
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/tests/queries/0_stateless/01677_bit_float.sql b/tests/queries/0_stateless/01677_bit_float.sql
new file mode 100644
index 00000000000..3692d8ac6a5
--- /dev/null
+++ b/tests/queries/0_stateless/01677_bit_float.sql
@@ -0,0 +1,9 @@
+SELECT bitAnd(0, inf); -- { serverError 43 }
+SELECT bitXor(0, inf); -- { serverError 43 }
+SELECT bitOr(0, inf); -- { serverError 43 }
+SELECT bitTest(inf, 0); -- { serverError 43 }
+SELECT bitTest(0, inf); -- { serverError 43 }
+SELECT bitRotateLeft(inf, 0); -- { serverError 43 }
+SELECT bitRotateRight(inf, 0); -- { serverError 43 }
+SELECT bitShiftLeft(inf, 0); -- { serverError 43 }
+SELECT bitShiftRight(inf, 0); -- { serverError 43 }