[Libosinfo] [PATCH v3 58/60] loader: sanity check entity filenames when loading
Christophe Fergeau
cfergeau at redhat.com
Fri Oct 16 10:48:02 UTC 2015
On Mon, Oct 12, 2015 at 06:11:19PM +0100, Daniel P. Berrange wrote:
> When we have loaded the id attribute of the entity, check it
> against the entity filename to ensure its XML document was
> held in a file with the modern naming.
>
> This merely prints out a warning for now to avoid breaking
> existing users, but in the future, files with non-compliant
> names will not be loaded at all.
>
> Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> ---
> osinfo/osinfo_loader.c | 105 +++++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 97 insertions(+), 8 deletions(-)
>
> diff --git a/osinfo/osinfo_loader.c b/osinfo/osinfo_loader.c
> index f7b841d..8a21a1b 100644
> --- a/osinfo/osinfo_loader.c
> +++ b/osinfo/osinfo_loader.c
> @@ -30,6 +30,7 @@
> #include <gio/gio.h>
>
> #include <string.h>
> +#include <ctype.h>
> #include <libxml/parser.h>
> #include <libxml/tree.h>
> #include <libxml/xpath.h>
> @@ -487,7 +488,49 @@ static OsinfoInstallScript *osinfo_loader_get_install_script(OsinfoLoader *loade
> return script;
> }
>
> +
> +static gboolean osinfo_loader_check_id(const gchar *relpath,
> + const gchar *type,
> + const gchar *id)
> +{
> + gchar *name;
> + gchar *suffix;
> + gboolean sep = FALSE;
> + gsize i;
> + if (g_str_has_prefix(id, "http://")) {
> + suffix = g_strdup(id + strlen("http://"));
> + } else {
> + suffix = g_strdup(id);
> + }
> + for (i = 0; suffix[i]; i++) {
> + if (suffix[i] == '/' && !sep) {
> + sep = TRUE;
> + } else if (!isalnum(suffix[i]) &&
> + suffix[i] != '-' &&
> + suffix[i] != '.' &&
> + suffix[i] != '_') {
> + suffix[i] = '-';
> + }
> + }
> + name = g_strdup_printf("/%s/%s.xml", type, suffix);
> + g_free(suffix);
> +
> + if (!g_str_equal(relpath, name)) {
> + g_warning("Entity %s should be in file %s not %s",
> + id, name, relpath);
name is leaked in this codepath.
> + return TRUE; /* In future switch to FALSE to refuse
> + * to load non-compliant named files.
> + * Need a period of grace for backcompat
> + * first though... Switch ETA Jan 2017
> + */
> + }
> + g_free(name);
> + return TRUE;
> +}
> +
> +
> static void osinfo_loader_device(OsinfoLoader *loader,
> + const gchar *relpath,
> xmlXPathContextPtr ctxt,
> xmlNodePtr root,
> GError **err)
> @@ -508,6 +551,10 @@ static void osinfo_loader_device(OsinfoLoader *loader,
> OSINFO_ERROR(err, _("Missing device id property"));
> return;
> }
> + if (!osinfo_loader_check_id(relpath, "device", id)) {
> + xmlFree(id);
> + return;
> + }
>
> OsinfoDevice *device = osinfo_loader_get_device(loader, id);
> g_hash_table_remove(loader->priv->entity_refs, id);
> @@ -652,6 +699,7 @@ static void osinfo_loader_product(OsinfoLoader *loader,
> }
>
> static void osinfo_loader_platform(OsinfoLoader *loader,
> + const gchar *relpath,
> xmlXPathContextPtr ctxt,
> xmlNodePtr root,
> GError **err)
> @@ -661,6 +709,10 @@ static void osinfo_loader_platform(OsinfoLoader *loader,
> OSINFO_ERROR(err, _("Missing platform id property"));
> return;
> }
> + if (!osinfo_loader_check_id(relpath, "platform", id)) {
> + xmlFree(id);
> + return;
> + }
>
> OsinfoPlatform *platform = osinfo_loader_get_platform(loader, id);
> g_hash_table_remove(loader->priv->entity_refs, id);
> @@ -681,6 +733,7 @@ static void osinfo_loader_platform(OsinfoLoader *loader,
> }
>
> static void osinfo_loader_deployment(OsinfoLoader *loader,
> + const gchar *relpath,
> xmlXPathContextPtr ctxt,
> xmlNodePtr root,
> GError **err)
> @@ -690,6 +743,10 @@ static void osinfo_loader_deployment(OsinfoLoader *loader,
> OSINFO_ERROR(err, _("Missing deployment id property"));
> return;
> }
> + if (!osinfo_loader_check_id(relpath, "deployment", id)) {
> + xmlFree(id);
> + return;
> + }
>
> gchar *osid = osinfo_loader_string("string(./os/@id)", loader, ctxt, err);
> if (!osid && 0) {
> @@ -730,6 +787,7 @@ static void osinfo_loader_deployment(OsinfoLoader *loader,
> }
>
> static void osinfo_loader_datamap(OsinfoLoader *loader,
> + const gchar *relpath,
> xmlXPathContextPtr ctxt,
> xmlNodePtr root,
> GError **err)
> @@ -744,6 +802,10 @@ static void osinfo_loader_datamap(OsinfoLoader *loader,
> OSINFO_ERROR(err, _("Missing os id property"));
> return;
> }
> + if (!osinfo_loader_check_id(relpath, "datamap", id)) {
> + xmlFree(id);
> + return;
> + }
>
> OsinfoDatamap *map = osinfo_loader_get_datamap(loader, id);
> g_hash_table_remove(loader->priv->entity_refs, id);
> @@ -836,6 +898,7 @@ static OsinfoAvatarFormat *osinfo_loader_avatar_format(OsinfoLoader *loader,
> }
>
> static void osinfo_loader_install_script(OsinfoLoader *loader,
> + const gchar *relpath,
> xmlXPathContextPtr ctxt,
> xmlNodePtr root,
> GError **err)
> @@ -864,6 +927,10 @@ static void osinfo_loader_install_script(OsinfoLoader *loader,
> return;
> }
>
> + if (!osinfo_loader_check_id(relpath, "install-script", id)) {
> + xmlFree(id);
> + return;
> + }
> OsinfoInstallScript *installScript = osinfo_loader_get_install_script(loader,
> id);
> g_hash_table_remove(loader->priv->entity_refs, id);
> @@ -1291,6 +1358,7 @@ static OsinfoDeviceDriver *osinfo_loader_driver(OsinfoLoader *loader,
>
>
> static void osinfo_loader_os(OsinfoLoader *loader,
> + const gchar *relpath,
> xmlXPathContextPtr ctxt,
> xmlNodePtr root,
> GError **err)
> @@ -1311,6 +1379,10 @@ static void osinfo_loader_os(OsinfoLoader *loader,
> OSINFO_ERROR(err, _("Missing os id property"));
> return;
> }
> + if (!osinfo_loader_check_id(relpath, "os", id)) {
> + xmlFree(id);
> + return;
> + }
>
> OsinfoOs *os = osinfo_loader_get_os(loader, id);
> g_hash_table_remove(loader->priv->entity_refs, id);
> @@ -1459,6 +1531,7 @@ cleanup:
> }
>
> static void osinfo_loader_root(OsinfoLoader *loader,
> + const gchar *relpath,
> xmlXPathContextPtr ctxt,
> xmlNodePtr root,
> GError **err)
> @@ -1494,22 +1567,22 @@ static void osinfo_loader_root(OsinfoLoader *loader,
> ctxt->node = it;
>
> if (xmlStrEqual(it->name, BAD_CAST "device"))
> - osinfo_loader_device(loader, ctxt, it, err);
> + osinfo_loader_device(loader, relpath, ctxt, it, err);
>
> else if (xmlStrEqual(it->name, BAD_CAST "platform"))
> - osinfo_loader_platform(loader, ctxt, it, err);
> + osinfo_loader_platform(loader, relpath, ctxt, it, err);
>
> else if (xmlStrEqual(it->name, BAD_CAST "os"))
> - osinfo_loader_os(loader, ctxt, it, err);
> + osinfo_loader_os(loader, relpath, ctxt, it, err);
>
> else if (xmlStrEqual(it->name, BAD_CAST "deployment"))
> - osinfo_loader_deployment(loader, ctxt, it, err);
> + osinfo_loader_deployment(loader, relpath, ctxt, it, err);
>
> else if (xmlStrEqual(it->name, BAD_CAST "install-script"))
> - osinfo_loader_install_script(loader, ctxt, it, err);
> + osinfo_loader_install_script(loader, relpath, ctxt, it, err);
>
> else if (xmlStrEqual(it->name, BAD_CAST "datamap"))
> - osinfo_loader_datamap(loader, ctxt, it, err);
> + osinfo_loader_datamap(loader, relpath, ctxt, it, err);
>
> ctxt->node = saved;
>
> @@ -1536,6 +1609,7 @@ catchXMLError(void *ctx, const char *msg ATTRIBUTE_UNUSED, ...)
> }
>
> static void osinfo_loader_process_xml(OsinfoLoader *loader,
> + const gchar *relpath,
> const gchar *xmlStr,
> const gchar *src,
> GError **err)
> @@ -1580,7 +1654,7 @@ static void osinfo_loader_process_xml(OsinfoLoader *loader,
>
> ctxt->node = root;
>
> - osinfo_loader_root(loader, ctxt, root, err);
> + osinfo_loader_root(loader, relpath, ctxt, root, err);
>
> cleanup:
> xmlXPathFreeContext(ctxt);
> @@ -1741,22 +1815,37 @@ osinfo_loader_process_file_reg_pci(OsinfoLoader *loader,
>
> static void
> osinfo_loader_process_file_reg_xml(OsinfoLoader *loader,
> + GFile *base,
> GFile *file,
> GError **err)
> {
> gchar *xml = NULL;
> gsize xmlLen;
> + gchar *basepath, *filepath;
> + const gchar *relpath;
> +
> g_file_load_contents(file, NULL, &xml, &xmlLen, NULL, err);
> if (error_is_set(err))
> return;
>
> + basepath = g_file_get_path(base);
> + filepath = g_file_get_path(file);
> + if (g_str_has_prefix(filepath, basepath)) {
> + relpath = filepath + strlen(basepath);
> + } else {
> + relpath = filepath;
> + g_warning("File %s does not have prefix %s", filepath, basepath);
> + }
g_file_get_relative_path() is going to do the hunk above for you.
> gchar *uri = g_file_get_uri(file);
> osinfo_loader_process_xml(loader,
> + relpath,
> xml,
> uri,
> err);
> g_free(uri);
> g_free(xml);
> + g_free(filepath);
> + g_free(basepath);
> }
>
>
> @@ -1838,7 +1927,7 @@ static void osinfo_loader_process_list(OsinfoLoader *loader,
>
> tmp = files;
> while (tmp) {
> - osinfo_loader_process_file_reg_xml(loader, tmp->data, &lerr);
> + osinfo_loader_process_file_reg_xml(loader, *dirs, tmp->data, &lerr);
> if (lerr) {
> g_propagate_error(err, lerr);
> break;
> --
> 2.4.3
>
> _______________________________________________
> Libosinfo mailing list
> Libosinfo at redhat.com
> https://www.redhat.com/mailman/listinfo/libosinfo
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libosinfo/attachments/20151016/7d360682/attachment.sig>
More information about the Libosinfo
mailing list