Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 195fda73 authored by Kevin Rocard's avatar Kevin Rocard Committed by android-build-team Robot
Browse files

Effect config parser: fix use after free on file path 



ParsingResult::configPath is the path of the configuration file
used for the factory config parsing.
This path is used for an error log if the configuration file has errors.

The paths used to be a static string literals stored as char*
without lifecycle management.
When it was changed to dynamic strings, the code was not updated.

This patch changes it to a std::string.

Bug: 111261328
Bug: 117118292
Test: flash and check effect works
Change-Id: Ia2022c794936f3f75793371cdde86c3047bb6c0a
Signed-off-by: default avatarKevin Rocard <krocard@google.com>
(cherry picked from commit a82fd600)
(cherry picked from commit 2fae5b7c)
parent 59a20586

media/libeffects/config/include/media/EffectsConfig.h

+1 −1
Original line number Original line Diff line number Diff line
@@ -96,7 +96,7 @@ struct ParsingResult { 
    /** Parsed config, nullptr if the xml lib could not load the file */

    /** Parsed config, nullptr if the xml lib could not load the file */

    std::unique_ptr<Config> parsedConfig;

    std::unique_ptr<Config> parsedConfig;

    size_t nbSkippedElement; //< Number of skipped invalid library, effect or processing chain

    size_t nbSkippedElement; //< Number of skipped invalid library, effect or processing chain

    const char* configPath; //< Path to the loaded configuration

    const std::string configPath; //< Path to the loaded configuration

};

};





/** Parses the provided effect configuration.

/** Parses the provided effect configuration.

media/libeffects/config/src/EffectsConfig.cpp

+8 −8
Original line number Original line Diff line number Diff line
@@ -250,14 +250,14 @@ bool parseStream(const XMLElement& xmlStream, Effects& effects, std::vector<Stre 
    return true;

    return true;

}

}





/** Internal version of the public parse(const char* path) with precondition `path != nullptr`. */

/** Internal version of the public parse(const char* path) where path always exist. */

ParsingResult parseWithPath(const char* path) {

ParsingResult parseWithPath(std::string&& path) {

    XMLDocument doc;

    XMLDocument doc;

    doc.LoadFile(path);

    doc.LoadFile(path.c_str());

    if (doc.Error()) {

    if (doc.Error()) {

        ALOGE("Failed to parse %s: Tinyxml2 error (%d): %s", path,

        ALOGE("Failed to parse %s: Tinyxml2 error (%d): %s", path.c_str(),

              doc.ErrorID(), doc.ErrorStr());

              doc.ErrorID(), doc.ErrorStr());

        return {nullptr, 0, path};

        return {nullptr, 0, std::move(path)};

    }

    }





    auto config = std::make_unique<Config>();

    auto config = std::make_unique<Config>();
@@ -295,7 +295,7 @@ ParsingResult parseWithPath(const char* path) { 
            }

            }

        }

        }

    }

    }

    return {std::move(config), nbSkippedElements, path};

    return {std::move(config), nbSkippedElements, std::move(path)};

}

}





}; // namespace

}; // namespace
@@ -310,14 +310,14 @@ ParsingResult parse(const char* path) { 
        if (access(defaultPath.c_str(), R_OK) != 0) {

        if (access(defaultPath.c_str(), R_OK) != 0) {

            continue;

            continue;

        }

        }

        auto result = parseWithPath(defaultPath.c_str());

        auto result = parseWithPath(std::move(defaultPath));

        if (result.parsedConfig != nullptr) {

        if (result.parsedConfig != nullptr) {

            return result;

            return result;

        }

        }

    }

    }





    ALOGE("Could not parse effect configuration in any of the default locations.");

    ALOGE("Could not parse effect configuration in any of the default locations.");

    return {nullptr, 0, nullptr};

    return {nullptr, 0, ""};

}

}





} // namespace effectsConfig

} // namespace effectsConfig

media/libeffects/factory/EffectsXmlConfigLoader.cpp

+2 −1
Original line number Original line Diff line number Diff line
@@ -327,7 +327,8 @@ extern "C" ssize_t EffectLoadXmlEffectConfig(const char* path) 
                                           &gSkippedEffects, &gSubEffectList);

                                           &gSkippedEffects, &gSubEffectList);





    ALOGE_IF(result.nbSkippedElement != 0, "%zu errors during loading of configuration: %s",

    ALOGE_IF(result.nbSkippedElement != 0, "%zu errors during loading of configuration: %s",

             result.nbSkippedElement, result.configPath ?: "No config file found");

             result.nbSkippedElement,

             result.configPath.empty() ? "No config file found" : result.configPath.c_str());





    return result.nbSkippedElement;

    return result.nbSkippedElement;

}

}