* #13534: account: add proper validation for video codecs

daemon/src/account.cpp

@@ -38,6 +38,7 @@
 #include "video/libav_utils.h"
 #endif
 
+#include "logger.h"
 #include "manager.h"
 #include "dbus/configurationmanager.h"
 
@@ -116,17 +117,77 @@ void Account::loadDefaultCodecs()
 
     setActiveAudioCodecs(result);
 #ifdef SFL_VIDEO
-    setVideoCodecs(libav_utils::getDefaultCodecs());
+    // we don't need to validate via setVideoCodecs, since these are defaults
+    videoCodecList_ = libav_utils::getDefaultCodecs();
 #endif
 }
 
+#ifdef SFL_VIDEO
+namespace {
+    bool isPositiveInteger(const string &s)
+    {
+        string::const_iterator it = s.begin();
+        while (it != s.end() and std::isdigit(*it))
+            ++it;
+        return not s.empty() and it == s.end();
+    }
+
+    bool isBoolean(const string &s)
+    {
+        return s == "true" or s == "false";
+    }
+
+    template <typename Predicate>
+    bool isFieldValid(const map<string, string> &codec, const char *field, Predicate p)
+    {
+        map<string, string>::const_iterator key(codec.find(field));
+        return key != codec.end() and p(key->second);
+    }
+
+    bool isCodecValid(const map<string, string> &codec, const vector<map<string, string> > &defaults)
+    {
+        map<string, string>::const_iterator name(codec.find(Account::VIDEO_CODEC_NAME));
+        if (name == codec.end()) {
+            ERROR("Field \"name\" missing in codec specification");
+            return false;
+        }
+
+        // check that it's in the list of valid codecs and that it has all the required fields
+        for (vector<map<string, string> >::const_iterator i = defaults.begin(); i != defaults.end(); ++i) {
+            map<string, string>::const_iterator defaultName = i->find(Account::VIDEO_CODEC_NAME);
+            if (defaultName->second == name->second) {
+                return isFieldValid(codec, Account::VIDEO_CODEC_BITRATE, isPositiveInteger)
+                    and isFieldValid(codec, Account::VIDEO_CODEC_ENABLED, isBoolean);
+            }
+        }
+        ERROR("Codec %s not supported", name->second.c_str());
+        return false;
+    }
+
+    bool isCodecListValid(const vector<map<string, string> > &list)
+    {
+        const vector<map<string, string> > defaults(libav_utils::getDefaultCodecs());
+        if (list.size() != defaults.size()) {
+            ERROR("New codec list has a different length than the list of supported codecs");
+            return false;
+        }
+
+        // make sure that all codecs are present
+        for (vector<map<string, string> >::const_iterator i = list.begin();
+             i != list.end(); ++i) {
+            if (not isCodecValid(*i, defaults))
+                return false;
+        }
+        return true;
+    }
+}
+#endif
+
 void Account::setVideoCodecs(const vector<map<string, string> > &list)
 {
 #ifdef SFL_VIDEO
-    // first clear the previously stored codecs
-    videoCodecList_.clear();
-    // FIXME: do real validation here
-    videoCodecList_ = list;
+    if (isCodecListValid(list))
+        videoCodecList_ = list;
 #else
     (void) list;
 #endif

daemon/src/account.h

@@ -184,6 +184,8 @@ class Account : public Serializable {
         }
 
         static const char * const VIDEO_CODEC_ENABLED;
+        static const char * const VIDEO_CODEC_NAME;
+        static const char * const VIDEO_CODEC_BITRATE;
     private:
         NON_COPYABLE(Account);
 
@@ -197,8 +199,6 @@ class Account : public Serializable {
         // General configuration keys for accounts
         static const char * const AUDIO_CODECS_KEY;
         static const char * const VIDEO_CODECS_KEY;
-        static const char * const VIDEO_CODEC_NAME;
-        static const char * const VIDEO_CODEC_BITRATE;
         static const char * const RINGTONE_PATH_KEY;
         static const char * const RINGTONE_ENABLED_KEY;
         static const char * const DISPLAY_NAME_KEY;

daemon/src/sip/sipaccount.cpp

@@ -317,7 +317,7 @@ void SIPAccount::unserialize(const Conf::MappingNode &mapNode)
         if (seq->empty()) {
             // Video codecs are an empty list
             WARN("Loading default video codecs");
-            setVideoCodecs(libav_utils::getDefaultCodecs());
+            videoCodecList_ = libav_utils::getDefaultCodecs();
         } else {
             vector<map<string, string> > videoCodecDetails;
             for (Sequence::iterator it = seq->begin(); it != seq->end(); ++it) {
@@ -328,13 +328,14 @@ void SIPAccount::unserialize(const Conf::MappingNode &mapNode)
                 codec->getValue(VIDEO_CODEC_ENABLED, &codecMap[VIDEO_CODEC_ENABLED]);
                 videoCodecDetails.push_back(codecMap);
             }
+            // these must be validated
             setVideoCodecs(videoCodecDetails);
         }
     } else {
         // either this is an older config file which had videoCodecs as a scalar node,
         // or it had no video codecs at all
         WARN("Loading default video codecs");
-        setVideoCodecs(libav_utils::getDefaultCodecs());
+        videoCodecList_ = libav_utils::getDefaultCodecs();
     }
 #endif
 

gnome/src/config/videoconf.c

@@ -366,10 +366,12 @@ bitrate_edited_cb(GtkCellRenderer *renderer UNUSED, gchar *path, gchar *new_text
     // Get active value and name at iteration
     const gint base = 10;
     gchar *endptr;
-    const unsigned long long val = strtoull(new_text, &endptr, base);
+    const long long val = strtoll(new_text, &endptr, base);
     /* Ignore if it's not a number */
     if (*endptr != '\0') {
         WARN("Ignoring characters %s\n", val, endptr);
+    } else if (val < 0) {
+        WARN("Ignoring negative bitrate value");
     } else {
         // Get path of edited codec
         GtkTreePath *tree_path = gtk_tree_path_new_from_string(path);