Merge pull request #3264 from rouault/fix_ossfuzz_49256

createOperations(): avoid potential infinite recursions (fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=49256)
OSGeo · Jul 20, 2022 · 6109096 · 6109096
2 parents 5b2d59b + 2f681ec
commit 6109096
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 3 deletions.
diff --git a/src/iso19111/common.cpp b/src/iso19111/common.cpp
@@ -443,8 +443,14 @@ bool Measure::_isEquivalentTo(const Measure &other,
     if (criterion == util::IComparable::Criterion::STRICT) {
         return operator==(other);
     }
-    return std::fabs(getSIValue() - other.getSIValue()) <=
-           maxRelativeError * std::fabs(getSIValue());
+    const double SIValue = getSIValue();
+    const double otherSIValue = other.getSIValue();
+    // It is arguable that we have to deal with infinite values, but this
+    // helps robustify some situations.
+    if (std::isinf(SIValue) && std::isinf(otherSIValue))
+        return SIValue * otherSIValue > 0;
+    return std::fabs(SIValue - otherSIValue) <=
+           maxRelativeError * std::fabs(SIValue);
 }
 
 // ---------------------------------------------------------------------------

diff --git a/src/iso19111/operation/coordinateoperationfactory.cpp b/src/iso19111/operation/coordinateoperationfactory.cpp
@@ -481,6 +481,7 @@ struct CoordinateOperationFactory::Private {
         bool inCreateOperationsGeogToVertWithAlternativeGeog = false;
         bool inCreateOperationsGeogToVertWithIntermediateVert = false;
         bool skipHorizontalTransformation = false;
+        int nRecLevelCreateOperations = 0;
         std::map<std::pair<io::AuthorityFactory::ObjectType, std::string>,
                  std::list<std::pair<std::string, std::string>>>
             cacheNameToCRS{};
@@ -3031,6 +3032,31 @@ CoordinateOperationFactory::Private::createOperations(
                 objectAsStr(targetCRS.get()) + ")");
 #endif
 
+#ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
+    // 10 is arbitrary and hopefully large enough for all transformations PROJ
+    // can handle.
+    // At time of writing 7 is the maximum known to be required by a few tests
+    // like
+    // operation.compoundCRS_to_compoundCRS_with_bound_crs_in_horiz_and_vert_WKT1_same_geoidgrids_context
+    // We don't enable that check for fuzzing, to be able to detect
+    // the root cause of recursions.
+    if (context.nRecLevelCreateOperations == 10) {
+        throw InvalidOperation("Too deep recursion in createOperations()");
+    }
+#endif
+
+    struct RecLevelIncrementer {
+        Private::Context &context_;
+
+        explicit inline RecLevelIncrementer(Private::Context &contextIn)
+            : context_(contextIn) {
+            ++context_.nRecLevelCreateOperations;
+        }
+
+        inline ~RecLevelIncrementer() { --context_.nRecLevelCreateOperations; }
+    };
+    RecLevelIncrementer recLevelIncrementer(context);
+
     std::vector<CoordinateOperationNNPtr> res;
 
     auto boundSrc = dynamic_cast<const crs::BoundCRS *>(sourceCRS.get());

diff --git a/test/unit/test_common.cpp b/test/unit/test_common.cpp
@@ -33,6 +33,8 @@
 #include "proj/metadata.hpp"
 #include "proj/util.hpp"
 
+#include <limits>
+
 using namespace osgeo::proj::common;
 using namespace osgeo::proj::metadata;
 using namespace osgeo::proj::operation;
@@ -69,7 +71,23 @@ TEST(common, unit_of_measure) {
 
 // ---------------------------------------------------------------------------
 
-TEST(common, measure) { EXPECT_TRUE(Measure(1.0) == Measure(1.0)); }
+TEST(common, measure) {
+    EXPECT_TRUE(Measure(0.0) == Measure(0.0));
+    EXPECT_TRUE(Measure(1.0) == Measure(1.0));
+    EXPECT_FALSE(Measure(1.0) == Measure(2.0));
+    EXPECT_FALSE(Measure(1.0) == Measure(0.0));
+    EXPECT_FALSE(Measure(0.0) == Measure(1.0));
+    EXPECT_TRUE(Measure(std::numeric_limits<double>::infinity()) ==
+                Measure(std::numeric_limits<double>::infinity()));
+    EXPECT_TRUE(Measure(-std::numeric_limits<double>::infinity()) ==
+                Measure(-std::numeric_limits<double>::infinity()));
+    EXPECT_FALSE(Measure(std::numeric_limits<double>::infinity()) ==
+                 Measure(-std::numeric_limits<double>::infinity()));
+    EXPECT_FALSE(Measure(std::numeric_limits<double>::infinity()) ==
+                 Measure(1.0));
+    EXPECT_FALSE(Measure(1.0) ==
+                 Measure(std::numeric_limits<double>::infinity()));
+}
 
 // ---------------------------------------------------------------------------