PFW: overflow not detected for int parameters

BZ: 26285

The following errors were not detected by the PFW when setting parameters
of type (U)INT8, (U)INT16, (U)INT32:
- When setting a value out of the int32 range (ex: 999999999999999), the
strtol/strtoul functions return the value -1 which was then assumed correct
by the PFW. Now the errno value is checked to ensure that no range error
was encountered by strtol/strtoul.
- When the input string does not contain any digits, the strtol/strtoul
functions return 0 which was assumed correct by the PFW. Now the endptr
argument is checked to make sure that at least a part of the string was
parsed.

In any case an error message is displayed and the original value is not
updated.

Made the change compliant to 64-bit OSes.

Applied the same corrections to Enum and FixedPoint types.

Change-Id: I135538def791208a6eb6143444a3fc30337137e1
Orig-Change-Id: I1519dbf798228a9be579aaf612f456d5eb1b41b5
Signed-off-by: Frederic Boisnard <fredericx.boisnard@intel.com>
Reviewed-on: http://android.intel.com:8080/55443
Reviewed-by: Mendi, EduardoX <eduardox.mendi@intel.com>
Tested-by: Mendi, EduardoX <eduardox.mendi@intel.com>
Reviewed-by: buildbot <buildbot@intel.com>
Tested-by: buildbot <buildbot@intel.com>

8 files changed, 199 insertions, 89 deletions

diff --git a/parameter/EnumParameterType.cpp b/parameter/EnumParameterType.cpp
index 60c937c..fc2123a 100644
--- a/parameter/EnumParameterType.cpp
+++ b/parameter/EnumParameterType.cpp
@@ -36,6 +36,7 @@
 #include <assert.h>
 #include "ParameterAccessContext.h"
 #include "EnumValuePair.h"
+#include <errno.h>
 
 #define base CParameterType
 
@@ -91,7 +92,7 @@ bool CEnumParameterType::fromXml(const CXmlElement& xmlElement, CXmlSerializingC
 // Conversion (tuning)
 bool CEnumParameterType::toBlackboard(const string& strValue, uint32_t& uiValue, CParameterAccessContext& parameterAccessContext) const
 {
-    int32_t iData;
+    int64_t iData;
 
     if (isNumber(strValue)) {
 
@@ -100,18 +101,34 @@ bool CEnumParameterType::toBlackboard(const string& strValue, uint32_t& uiValue,
         // Hexa
         bool bValueProvidedAsHexa = !strValue.compare(0, 2, "0x");
 
+        errno = 0;
+        char *pcStrEnd;
+
         // Get value
-        iData = strtol(strValue.c_str(), NULL, 0);
+        iData = strtoll(strValue.c_str(), &pcStrEnd, 0);
+
+        // Conversion error when the input string does not contain any digit or the number is out of range (int32_t type)
+        bool bConversionSucceeded = !errno && (strValue.c_str() != pcStrEnd);
+
+
+        if (!bConversionSucceeded || !isEncodable((uint64_t)iData, !bValueProvidedAsHexa)) {
+
+            // Illegal value provided
+            parameterAccessContext.setError("Provided value out of bound");
 
-        if (bValueProvidedAsHexa && isEncodable(iData)) {
+            return false;
+        }
+
+        if (bValueProvidedAsHexa) {
 
             // Sign extend
             signExtend(iData);
         }
         // Check validity
-        if (!isValid(iData)) {
+        string strError;
+        if (!isValid(iData, parameterAccessContext)) {
 
-            parameterAccessContext.setError("Provided value not part of numerical space");
+            parameterAccessContext.setError(strError);
 
             return false;
         }
@@ -119,16 +136,18 @@ bool CEnumParameterType::toBlackboard(const string& strValue, uint32_t& uiValue,
         // Literal value provided
 
         // Check validity
-        if (!getNumerical(strValue, iData)) {
+        int iNumerical;
+        if (!getNumerical(strValue, iNumerical)) {
 
             parameterAccessContext.setError("Provided value not part of lexical space");
 
             return false;
         }
+        iData = iNumerical;
     }
 
     // Return data
-    uiValue = iData;
+    uiValue = (uint32_t)iData;
 
     return true;
 }
@@ -177,9 +196,7 @@ bool CEnumParameterType::fromBlackboard(string& strValue, const uint32_t& uiValu
 // Value access
 bool CEnumParameterType::toBlackboard(int32_t iUserValue, uint32_t& uiValue, CParameterAccessContext& parameterAccessContext) const
 {
-    if (!isValid(iUserValue)) {
-
-        parameterAccessContext.setError("Invalid value");
+    if (!isValid(iUserValue, parameterAccessContext)) {
 
         return false;
     }
@@ -243,7 +260,7 @@ bool CEnumParameterType::getLiteral(int32_t iNumerical, string& strLiteral) cons
     return false;
 }
 
-bool CEnumParameterType::getNumerical(const string& strLiteral, int32_t& iNumerical) const
+bool CEnumParameterType::getNumerical(const string& strLiteral, int& iNumerical) const
 {
     uint32_t uiChild;
     uint32_t uiNbChildren = getNbChildren();
@@ -263,9 +280,10 @@ bool CEnumParameterType::getNumerical(const string& strLiteral, int32_t& iNumeri
     return false;
 }
 
-// Numerical validity
-bool CEnumParameterType::isValid(int32_t iNumerical) const
+// Numerical validity of the enum value
+bool CEnumParameterType::isValid(int iNumerical, CParameterAccessContext& parameterAccessContext) const
 {
+    // Check that the value is part of the allowed values for this kind of enum
     uint32_t uiChild;
     uint32_t uiNbChildren = getNbChildren();
 
@@ -279,5 +297,7 @@ bool CEnumParameterType::isValid(int32_t iNumerical) const
         }
     }
 
+    parameterAccessContext.setError("Provided value not part of numerical space");
+
     return false;
 }
diff --git a/parameter/EnumParameterType.h b/parameter/EnumParameterType.h
index a195fea..f63e7ac 100644
--- a/parameter/EnumParameterType.h
+++ b/parameter/EnumParameterType.h
@@ -66,8 +66,8 @@ private:
 
     // Literal - numerical conversions
     bool getLiteral(int32_t iNumerical, string& strLiteral) const;
-    bool getNumerical(const string& strLiteral, int32_t& iNumerical) const;
+    bool getNumerical(const string& strLiteral, int& iNumerical) const;
 
     // Numerical validity
-    bool isValid(int32_t iNumerical) const;
+    bool isValid(int iNumerical, CParameterAccessContext& parameterAccessContext) const;
 };
diff --git a/parameter/FixedPointParameterType.cpp b/parameter/FixedPointParameterType.cpp
index 63d4d4c..0580c55 100644
--- a/parameter/FixedPointParameterType.cpp
+++ b/parameter/FixedPointParameterType.cpp
@@ -37,6 +37,7 @@
 #include "Parameter.h"
 #include "ParameterAccessContext.h"
 #include "ConfigurationAccessContext.h"
+#include <errno.h>
 
 #define base CParameterType
 
@@ -122,36 +123,57 @@ bool CFixedPointParameterType::toBlackboard(const string& strValue, uint32_t& ui
         return false;
     }
 
-    int32_t iData;
+    int64_t iData;
 
     if (parameterAccessContext.valueSpaceIsRaw()) {
+        errno = 0;
+        char *pcStrEnd;
 
         // Get data in integer form
-        iData = strtol(strValue.c_str(), NULL, 0);
+        iData = strtoll(strValue.c_str(), &pcStrEnd, 0);
 
-        if (bValueProvidedAsHexa && isEncodable(iData)) {
+        // Conversion error when the input string does not contain any digit or the number is out of range
+        bool bConversionSucceeded = !errno && (strValue.c_str() != pcStrEnd);
+
+        if (!bConversionSucceeded || !isEncodable((uint64_t)iData, !bValueProvidedAsHexa)) {
+
+            // Illegal value provided
+            parameterAccessContext.setError(getOutOfRangeError(strValue, parameterAccessContext.valueSpaceIsRaw(), bValueProvidedAsHexa));
+
+            return false;
+        }
+        if (bValueProvidedAsHexa) {
 
             // Sign extend
             signExtend(iData);
         }
 
     } else {
-        double dData = strtod(strValue.c_str(), NULL);
+        errno = 0;
+        char *pcStrEnd;
 
-        // Do the conversion
-        iData = asInteger(dData);
-    }
+        double dData = strtod(strValue.c_str(), &pcStrEnd);
 
-    // Check integrity
-    if (!isConsistent(iData)) {
+        // Conversion error when the input string does not contain any digit or the number is out of range (int32_t type)
+        bool bConversionSucceeded = !errno && (strValue.c_str() != pcStrEnd);
 
-        // Illegal value provided
-        parameterAccessContext.setError(getOutOfRangeError(strValue, parameterAccessContext.valueSpaceIsRaw(), bValueProvidedAsHexa));
+        // Check encodability
+        if (!bConversionSucceeded || !checkValueAgainstRange(dData)) {
 
-        return false;
+            // Illegal value provided
+            parameterAccessContext.setError(getOutOfRangeError(strValue, parameterAccessContext.valueSpaceIsRaw(), bValueProvidedAsHexa));
+
+            return false;
+        }
+
+        // Do the conversion
+        iData = asInteger(dData);
     }
 
-    uiValue = iData;
+    // check that the data is encodable and can be safely written to the blackboard
+    assert(isEncodable((unsigned long int)iData, true));
+
+    uiValue = (uint32_t)iData;
 
     return true;
 }
@@ -160,8 +182,8 @@ bool CFixedPointParameterType::fromBlackboard(string& strValue, const uint32_t&
 {
     int32_t iData = uiValue;
 
-    // Check consistency
-    assert(isEncodable(iData));
+    // Check encodability
+    assert(isEncodable((uint32_t)iData, false));
 
     // Format
     ostringstream strStream;
@@ -202,11 +224,8 @@ bool CFixedPointParameterType::fromBlackboard(string& strValue, const uint32_t&
 // Value access
 bool CFixedPointParameterType::toBlackboard(double dUserValue, uint32_t& uiValue, CParameterAccessContext& parameterAccessContext) const
 {
-    // Do the conversion
-    int32_t iData = asInteger(dUserValue);
-
-    // Check integrity
-    if (!isConsistent(iData)) {
+    // Check that the value is within the allowed range for this type
+    if (!checkValueAgainstRange(dUserValue)) {
 
         // Illegal value provided
         parameterAccessContext.setError("Value out of range");
@@ -214,6 +233,12 @@ bool CFixedPointParameterType::toBlackboard(double dUserValue, uint32_t& uiValue
         return false;
     }
 
+    // Do the conversion
+    int32_t iData = asInteger(dUserValue);
+
+    // Check integrity
+    assert(isEncodable((uint32_t)iData, true));
+
     uiValue = iData;
 
     return true;
@@ -225,8 +250,8 @@ bool CFixedPointParameterType::fromBlackboard(double& dUserValue, uint32_t uiVal
 
     int32_t iData = uiValue;
 
-    // Check consistency
-    assert(isEncodable(iData));
+    // Check unsigned value is encodable
+    assert(isEncodable(uiValue, false));
 
     // Sign extend
     signExtend(iData);
@@ -242,12 +267,17 @@ uint32_t CFixedPointParameterType::getUtilSizeInBits() const
     return _uiIntegral + _uiFractional + 1;
 }
 
+// Compute the range for the type (minimum and maximum values)
+void CFixedPointParameterType::getRange(double& dMin, double& dMax) const
+{
+    dMax = (double)((1UL << (_uiIntegral + _uiFractional)) - 1) / (1UL << _uiFractional);
+    dMin = -(double)(1UL << (_uiIntegral + _uiFractional)) / (1UL << _uiFractional);
+}
+
 // Out of range error
 string CFixedPointParameterType::getOutOfRangeError(const string& strValue, bool bRawValueSpace, bool bHexaValue) const
 {
-    // Min/Max computation
-    int32_t iMax = (1L << (getSize() * 8 - 1)) - 1;
-    int32_t iMin = -iMax - 1;
+
 
     ostringstream strStream;
 
@@ -255,9 +285,18 @@ string CFixedPointParameterType::getOutOfRangeError(const string& strValue, bool
 
     if (!bRawValueSpace) {
 
-        strStream << "real range [" << (double)iMin / (1UL << _uiFractional) << ", "<< (double)iMax / (1UL << _uiFractional) << "]";
+        // Min/Max computation
+        double dMin = 0;
+        double dMax = 0;
+        getRange(dMin, dMax);
+
+        strStream << "real range [" << dMin << ", "<< dMax << "]";
     } else {
 
+        // Min/Max computation
+        int32_t iMax = (1L << (getSize() * 8 - 1)) - 1;
+        int32_t iMin = -iMax - 1;
+
         strStream << "raw range [";
 
         if (bHexaValue) {
@@ -279,21 +318,14 @@ string CFixedPointParameterType::getOutOfRangeError(const string& strValue, bool
     return strStream.str();
 }
 
-// Check data is consistent with available range, with respect to its sign
-bool CFixedPointParameterType::isConsistent(uint32_t uiData) const
+// Check that the value is within available range for this type
+bool CFixedPointParameterType::checkValueAgainstRange(double dValue) const
 {
-    uint32_t uiShift = getSize() * 8;
-
-    if (uiShift == 8 * sizeof(uiData)) {
-        // Prevent inappropriate shifts
-        return true;
-    }
-
-    // Negative value?
-    bool bIsValueExpectedNegative = (uiData & (1 << (uiShift - 1))) != 0;
+    double dMin = 0;
+    double dMax = 0;
+    getRange(dMin, dMax);
 
-    // Check high bits are clean
-    return bIsValueExpectedNegative ? !(~uiData >> uiShift) : !(uiData >> uiShift);
+    return (dValue <= dMax) && (dValue >= dMin);
 }
 
 // Data conversion
diff --git a/parameter/FixedPointParameterType.h b/parameter/FixedPointParameterType.h
index 4dcf85f..b3b7648 100644
--- a/parameter/FixedPointParameterType.h
+++ b/parameter/FixedPointParameterType.h
@@ -60,10 +60,12 @@ public:
 private:
     // Util size
     uint32_t getUtilSizeInBits() const;
+    // Range computation
+    void getRange(double& dMin, double& dMax) const;
     // Out of range error
     string getOutOfRangeError(const string& strValue, bool bRawValueSpace, bool bHexaValue) const;
-    // Check data is consistent with available range, with respect to its sign
-    bool isConsistent(uint32_t uiData) const;
+    // Check if data is encodable
+    bool checkValueAgainstRange(double dValue) const;
     // Data conversion
     int32_t asInteger(double dValue) const;
     double asDouble(int32_t iValue) const;
diff --git a/parameter/IntegerParameterType.cpp b/parameter/IntegerParameterType.cpp
index fb55f08..80d2c90 100644
--- a/parameter/IntegerParameterType.cpp
+++ b/parameter/IntegerParameterType.cpp
@@ -35,6 +35,7 @@
 #include "ParameterAccessContext.h"
 #include <assert.h>
 #include "ParameterAdaptation.h"
+#include <errno.h>
 
 #define base CParameterType
 
@@ -147,45 +148,55 @@ bool CIntegerParameterType::toBlackboard(const string& strValue, uint32_t& uiVal
     bool bValueProvidedAsHexa = !strValue.compare(0, 2, "0x");
 
     // Get value
-    int32_t iData;
+    int64_t iData;
 
+    // Reset errno to check if it is updated during the conversion (strtol/strtoul)
+    errno = 0;
+    char *pcStrEnd;
+
+    // Convert the input string
     if (_bSigned) {
 
-        iData = strtoul(strValue.c_str(), NULL, 0);
+        iData = strtoll(strValue.c_str(), &pcStrEnd, 0);
     } else {
 
-        iData = strtol(strValue.c_str(), NULL, 0);
+        iData = strtoull(strValue.c_str(), &pcStrEnd, 0);
     }
 
-    if (bValueProvidedAsHexa && isEncodable(iData)) {
 
-        // Sign extend
-        signExtend(iData);
-    }
+    // Conversion error when the input string does not contain any digit or the number is out of range (int32_t type)
+    bool bConversionSucceeded = !errno && (strValue.c_str() != pcStrEnd);
+
     // Check against Min / Max
     if (_bSigned) {
 
-        if (!checkValueAgainstRange<int32_t>(strValue, iData, parameterAccessContext, bValueProvidedAsHexa)) {
+        if (bConversionSucceeded && bValueProvidedAsHexa && isEncodable((uint64_t)iData, !bValueProvidedAsHexa)) {
+
+            // Sign extend
+            signExtend(iData);
+        }
+
+        if (!checkValueAgainstRange<int64_t>(strValue, iData, (int32_t)_uiMin, (int32_t)_uiMax, parameterAccessContext, bValueProvidedAsHexa, bConversionSucceeded)) {
 
             return false;
         }
     } else {
 
-        if (!checkValueAgainstRange<uint32_t>(strValue, iData, parameterAccessContext, bValueProvidedAsHexa)) {
+        if (!checkValueAgainstRange<uint64_t>(strValue, iData, _uiMin, _uiMax, parameterAccessContext, bValueProvidedAsHexa, bConversionSucceeded)) {
 
             return false;
         }
     }
 
-    uiValue = iData;
+    uiValue = (uint32_t)iData;
 
     return true;
 }
 
 bool CIntegerParameterType::fromBlackboard(string& strValue, const uint32_t& uiValue, CParameterAccessContext& parameterAccessContext) const
 {
-    // Check consistency
-    assert(isEncodable(uiValue));
+    // Check unsigned value is encodable
+    assert(isEncodable(uiValue, false));
 
     // Format
     ostringstream strStream;
@@ -371,9 +382,9 @@ uint32_t CIntegerParameterType::getDefaultValue() const
 }
 
 // Range checking
-template <typename type> bool CIntegerParameterType::checkValueAgainstRange(const string& strValue, type value, CParameterAccessContext& parameterAccessContext, bool bHexaValue) const
+template <typename type> bool CIntegerParameterType::checkValueAgainstRange(const string& strValue, type value, type minValue, type maxValue, CParameterAccessContext& parameterAccessContext, bool bHexaValue, bool bConversionSucceeded) const
 {
-    if ((type)value < (type)_uiMin || (type)value > (type)_uiMax) {
+    if (!bConversionSucceeded || value < minValue || value > maxValue) {
 
         ostringstream strStream;
 
@@ -382,13 +393,13 @@ template <typename type> bool CIntegerParameterType::checkValueAgainstRange(cons
         if (bHexaValue) {
 
             // Format Min
-            strStream << "0x" << hex << uppercase << setw(getSize()*2) << setfill('0') << makeEncodable(_uiMin);
+            strStream << "0x" << hex << uppercase << setw(getSize()*2) << setfill('0') << makeEncodable(minValue);
             // Format Max
-            strStream << ", 0x" << hex << uppercase << setw(getSize()*2) << setfill('0') << makeEncodable(_uiMax);
+            strStream << ", 0x" << hex << uppercase << setw(getSize()*2) << setfill('0') << makeEncodable(maxValue);
 
         } else {
 
-            strStream << (type)_uiMin << ", " <<  (type)_uiMax;
+            strStream << minValue << ", " <<  maxValue;
         }
 
         strStream << "] for " << getKind();
diff --git a/parameter/IntegerParameterType.h b/parameter/IntegerParameterType.h
index e0b31a0..6f984f7 100644
--- a/parameter/IntegerParameterType.h
+++ b/parameter/IntegerParameterType.h
@@ -69,7 +69,7 @@ private:
     virtual bool childrenAreDynamic() const;
 
     // Range checking
-    template <typename type> bool checkValueAgainstRange(const string& strValue, type value, CParameterAccessContext& parameterAccessContext, bool bHexaValue) const;
+    template <typename type> bool checkValueAgainstRange(const string& strValue, type value, type minValue, type maxValue, CParameterAccessContext& parameterAccessContext, bool bHexaValue, bool bConversionSucceeded) const;
 
     // Adaptation element retrieval
     const CParameterAdaptation* getParameterAdaptation() const;
diff --git a/parameter/ParameterType.cpp b/parameter/ParameterType.cpp
index 7079a46..a271f73 100644
--- a/parameter/ParameterType.cpp
+++ b/parameter/ParameterType.cpp
@@ -97,7 +97,7 @@ void CParameterType::showProperties(string& strResult) const
     }
 
     // Scalar size
-    strResult += "Scalar size: " + toString(_uiSize) + " byte(s) \n";
+    strResult += "Scalar size: " + toString(getSize()) + " byte(s) \n";
 }
 
 // Default value handling (simulation only)
@@ -118,40 +118,77 @@ CInstanceConfigurableElement* CParameterType::doInstantiate() const
     }
 }
 
-// Sign extension
+// Sign extension (32 bits)
 void CParameterType::signExtend(int32_t& iData) const
 {
-    uint32_t uiSizeInBits = _uiSize * 8;
-    uint32_t uiShift = 8 * sizeof(iData) - uiSizeInBits;
+    doSignExtend(iData);
+}
+
+// Sign extension (64 bits)
+void CParameterType::signExtend(int64_t& iData) const
+{
+    doSignExtend(iData);
+}
+
+// Generic sign extension
+template <typename type>
+void CParameterType::doSignExtend(type& data) const
+{
+    uint32_t uiSizeInBits = getSize() * 8;
+    uint32_t uiShift = 8 * sizeof(data) - uiSizeInBits;
 
     if (uiShift) {
 
-        iData = (iData << uiShift) >> uiShift;
+        data = (data << uiShift) >> uiShift;
     }
 }
 
-// Check data has no bit set outside available range
-bool CParameterType::isEncodable(uint32_t uiData) const
+// Check data has no bit set outside available range (32 bits)
+bool CParameterType::isEncodable(uint32_t uiData, bool bIsSigned) const
 {
-    uint32_t uiSizeInBits = _uiSize * 8;
+    return doIsEncodable(uiData, bIsSigned);
+}
 
-    if (uiSizeInBits == 8 * sizeof(uiData)) {
+// Check data has no bit set outside available range (64 bits)
+bool CParameterType::isEncodable(uint64_t uiData, bool bIsSigned) const
+{
+    return doIsEncodable(uiData, bIsSigned);
+}
 
+// Generic encodability check
+template <typename type>
+bool CParameterType::doIsEncodable(type data, bool bIsSigned) const
+{
+    if (getSize() == sizeof(data)) {
+        // Prevent inappropriate shifts
         return true;
     }
 
-    // Check high bits are clean
-    return !(uiData >> uiSizeInBits);
+    uint32_t uiShift = getSize() * 8;
+
+    if (!bIsSigned) {
+
+        // Check high bits are clean
+        return !(data >> uiShift);
+
+    } else {
+
+        // Negative value?
+        bool bIsValueExpectedNegative = (data & (1 << (uiShift - 1))) != 0;
+
+        // Check high bits are clean
+        return bIsValueExpectedNegative ? !(~data >> uiShift) : !(data >> uiShift);
+    }
 }
 
 // Remove all bits set outside available range
 uint32_t CParameterType::makeEncodable(uint32_t uiData) const
 {
-    if (_uiSize == sizeof(uint32_t)) {
+    if (getSize() == sizeof(uint32_t)) {
 
         return uiData;
     }
-    uint32_t uiSizeInBits = _uiSize * 8;
+    uint32_t uiSizeInBits = getSize() * 8;
 
     uint32_t uiMask = (1 << uiSizeInBits) - 1;
 
diff --git a/parameter/ParameterType.h b/parameter/ParameterType.h
index 83e3c34..ef8ef67 100644
--- a/parameter/ParameterType.h
+++ b/parameter/ParameterType.h
@@ -85,14 +85,22 @@ protected:
     void setSize(uint32_t uiSize);
     // Sign extension
     void signExtend(int32_t& iData) const;
-    // Check data has no bit set outside available range (based on byte size)
-    bool isEncodable(uint32_t uiData) const;
+    void signExtend(int64_t& iData) const;
+    // Check data has no bit set outside available range (based on byte size) and
+    // check data is consistent with available range, with respect to its sign
+    bool isEncodable(uint32_t uiData, bool bIsSigned) const;
+    bool isEncodable(uint64_t uiData, bool bIsSigned) const;
     // Remove all bits set outside available range
     uint32_t makeEncodable(uint32_t uiData) const;
 
 private:
     // Instantiation
     virtual CInstanceConfigurableElement* doInstantiate() const;
+    // Generic Access
+    template <typename type>
+    void doSignExtend(type& data) const;
+    template <typename type>
+    bool doIsEncodable(type data, bool bIsSigned) const;
 
     // Size in bytes
     uint32_t _uiSize;