--- python/Makefile.am | 5 ++++ src/appliance-kcmdline.c | 21 ++----------- src/drives.c | 65 +++++++---------------------------------- src/guestfs-internal-frontend.h | 3 ++ src/unit-tests.c | 40 +++++++++++++++++++++++++ src/utils.c | 50 +++++++++++++++++++++++++++++-- 6 files changed, 110 insertions(+), 74 deletions(-) diff --git a/python/Makefile.am b/python/Makefile.am index 486b52e..10fa0d3 100644 --- a/python/Makefile.am +++ b/python/Makefile.am @@ -95,6 +95,7 @@ setup-install: setup.py stamp-extra-files # Python's crappy MANIFEST file cannot graft single files, so we have # to hard-link any extra files we need into the local directory. stamp-extra-files: \ + c-ctype.h \ config.h \ guestfs-internal-all.h \ guestfs-internal-frontend-cleanups.h \ @@ -106,6 +107,9 @@ stamp-extra-files: \ config.h: ln ../config.h $@ +c-ctype.h: + ln $(top_srcdir)/gnulib/lib/c-ctype.h $@ + ignore-value.h: ln $(top_srcdir)/gnulib/lib/ignore-value.h $@ @@ -138,6 +142,7 @@ CLEANFILES += \ *.pyc \ examples/*~ examples/*.pyc \ t/*~ t/*.pyc \ + c-ctype.h \ config.h \ guestfs-internal-all.h \ guestfs-internal-frontend-cleanups.h \ diff --git a/src/appliance-kcmdline.c b/src/appliance-kcmdline.c index 6771668..c5bc65f 100644 --- a/src/appliance-kcmdline.c +++ b/src/appliance-kcmdline.c @@ -36,23 +36,8 @@ * Check that the $TERM environment variable is reasonable before * we pass it through to the appliance. */ -static bool -valid_term (const char *term) -{ - size_t len = strlen (term); - - if (len == 0 || len > 16) - return false; - - while (len > 0) { - char c = *term++; - len--; - if (!c_isalnum (c) && c != '-' && c != '_') - return false; - } - - return true; -} +#define VALID_TERM(term) \ + guestfs_int_string_is_valid ((term), 1, 16, true, true, "-_") #if defined(__powerpc64__) #define SERIAL_CONSOLE "console=hvc0 console=ttyS0" @@ -196,7 +181,7 @@ guestfs_int_appliance_command_line (guestfs_h *g, const char *appliance_dev, guestfs_int_add_string (g, &argv, "guestfs_network=1"); /* TERM environment variable. */ - if (term && valid_term (term)) + if (term && VALID_TERM (term)) guestfs_int_add_sprintf (g, &argv, "TERM=%s", term); else guestfs_int_add_string (g, &argv, "TERM=linux"); diff --git a/src/drives.c b/src/drives.c index 1e04f81..594f019 100644 --- a/src/drives.c +++ b/src/drives.c @@ -588,65 +588,22 @@ guestfs_int_free_drives (guestfs_h *g) * Check string parameter matches regular expression * C<^[-_[:alnum:]]+$> (in C locale). */ -static int -valid_format_iface (const char *str) -{ - size_t len = strlen (str); - - if (len == 0) - return 0; - - while (len > 0) { - char c = *str++; - len--; - if (c != '-' && c != '_' && !c_isalnum (c)) - return 0; - } - return 1; -} +#define VALID_FORMAT_IFACE(str) \ + guestfs_int_string_is_valid ((str), 1, SIZE_MAX, true, true, "-_") /** * Check the disk label is reasonable. It can't contain certain * characters, eg. C<'/'>, C<','>. However be stricter here and * ensure it's just alphabetic and E<le> 20 characters in length. */ -static int -valid_disk_label (const char *str) -{ - size_t len = strlen (str); - - if (len == 0 || len > 20) - return 0; - - while (len > 0) { - char c = *str++; - len--; - if (!c_isalpha (c)) - return 0; - } - return 1; -} +#define VALID_DISK_LABEL(str) \ + guestfs_int_string_is_valid ((str), 1, 20, true, false, NULL) /** * Check the server hostname is reasonable. */ -static int -valid_hostname (const char *str) -{ - size_t len = strlen (str); - - if (len == 0 || len > 255) - return 0; - - while (len > 0) { - char c = *str++; - len--; - if (!c_isalnum (c) && - c != '-' && c != '.' && c != ':' && c != '[' && c != ']') - return 0; - } - return 1; -} +#define VALID_HOSTNAME(str) \ + guestfs_int_string_is_valid ((str), 1, 255, true, true, "-.:[]") /** * Check the port number is reasonable. @@ -700,7 +657,7 @@ parse_one_server (guestfs_h *g, const char *server, struct drive_server *ret) return -1; } free (port_str); - if (!valid_hostname (hostname)) { + if (!VALID_HOSTNAME (hostname)) { error (g, _("invalid hostname '%s'"), hostname); free (hostname); return -1; @@ -711,7 +668,7 @@ parse_one_server (guestfs_h *g, const char *server, struct drive_server *ret) } /* Doesn't match anything above, so assume it's a bare hostname. */ - if (!valid_hostname (server)) { + if (!VALID_HOSTNAME (server)) { error (g, _("invalid hostname or server string '%s'"), server); return -1; } @@ -814,19 +771,19 @@ guestfs_impl_add_drive_opts (guestfs_h *g, const char *filename, return -1; } - if (data.format && !valid_format_iface (data.format)) { + if (data.format && !VALID_FORMAT_IFACE (data.format)) { error (g, _("%s parameter is empty or contains disallowed characters"), "format"); free_drive_servers (data.servers, data.nr_servers); return -1; } - if (data.iface && !valid_format_iface (data.iface)) { + if (data.iface && !VALID_FORMAT_IFACE (data.iface)) { error (g, _("%s parameter is empty or contains disallowed characters"), "iface"); free_drive_servers (data.servers, data.nr_servers); return -1; } - if (data.disk_label && !valid_disk_label (data.disk_label)) { + if (data.disk_label && !VALID_DISK_LABEL (data.disk_label)) { error (g, _("label parameter is empty, too long, or contains disallowed characters")); free_drive_servers (data.servers, data.nr_servers); return -1; diff --git a/src/guestfs-internal-frontend.h b/src/guestfs-internal-frontend.h index bc57506..ebc1f1b 100644 --- a/src/guestfs-internal-frontend.h +++ b/src/guestfs-internal-frontend.h @@ -32,6 +32,8 @@ #ifndef GUESTFS_INTERNAL_FRONTEND_H_ #define GUESTFS_INTERNAL_FRONTEND_H_ +#include <stdbool.h> + #include "guestfs-internal-all.h" #define _(str) dgettext(PACKAGE, (str)) @@ -89,6 +91,7 @@ extern int guestfs_int_random_string (char *ret, size_t len); extern char *guestfs_int_drive_name (size_t index, char *ret); extern ssize_t guestfs_int_drive_index (const char *); extern int guestfs_int_is_true (const char *str); +extern bool guestfs_int_string_is_valid (const char *str, size_t min_length, size_t max_length, bool alpha, bool digit, const char *extra); //extern void guestfs_int_fadvise_normal (int fd); extern void guestfs_int_fadvise_sequential (int fd); extern void guestfs_int_fadvise_random (int fd); diff --git a/src/unit-tests.c b/src/unit-tests.c index 06d4d27..e22a61e 100644 --- a/src/unit-tests.c +++ b/src/unit-tests.c @@ -433,6 +433,45 @@ test_stringsbuf (void) guestfs_close (g); } +/* Use the same macros as in src/drives.c */ +#define VALID_FORMAT_IFACE(str) \ + guestfs_int_string_is_valid ((str), 1, SIZE_MAX, true, true, "-_") +#define VALID_DISK_LABEL(str) \ + guestfs_int_string_is_valid ((str), 1, 20, true, false, NULL) +#define VALID_HOSTNAME(str) \ + guestfs_int_string_is_valid ((str), 1, 255, true, true, "-.:[]") + +static void +test_valid (void) +{ + assert (!VALID_FORMAT_IFACE ("")); + assert (!VALID_DISK_LABEL ("")); + assert (!VALID_HOSTNAME ("")); + + assert (!VALID_DISK_LABEL ("012345678901234567890")); + + assert (VALID_FORMAT_IFACE ("abc")); + assert (VALID_FORMAT_IFACE ("ABC")); + assert (VALID_FORMAT_IFACE ("abc123")); + assert (VALID_FORMAT_IFACE ("abc123-")); + assert (VALID_FORMAT_IFACE ("abc123_")); + assert (!VALID_FORMAT_IFACE ("abc123.")); + + assert (VALID_DISK_LABEL ("abc")); + assert (VALID_DISK_LABEL ("ABC")); + assert (!VALID_DISK_LABEL ("abc123")); + assert (!VALID_DISK_LABEL ("abc123-")); + + assert (VALID_HOSTNAME ("abc")); + assert (VALID_HOSTNAME ("ABC")); + assert (VALID_HOSTNAME ("abc123")); + assert (VALID_HOSTNAME ("abc-123")); + assert (VALID_HOSTNAME ("abc.123")); + assert (VALID_HOSTNAME ("abc:123")); + assert (VALID_HOSTNAME ("abc[123]")); + assert (!VALID_HOSTNAME ("abc/def")); +} + int main (int argc, char *argv[]) { @@ -448,6 +487,7 @@ main (int argc, char *argv[]) test_timeval_diff (); test_match (); test_stringsbuf (); + test_valid (); exit (EXIT_SUCCESS); } diff --git a/src/utils.c b/src/utils.c index 0b8962c..6a01ac8 100644 --- a/src/utils.c +++ b/src/utils.c @@ -27,6 +27,7 @@ #include <stdio.h> #include <stdlib.h> +#include <stdbool.h> #include <string.h> #include <unistd.h> #include <fcntl.h> @@ -37,9 +38,11 @@ /* NB: MUST NOT require linking to gnulib, because that will break the * Python 'sdist' which includes a copy of this file. It's OK to - * include "ignore-value.h" here (since it is a header only with no - * other code), but we also had to copy this file to the Python sdist. + * include "c-ctype.h" and "ignore-value.h" here (since it is a header + * only with no other code), but we also had to copy these files to + * the Python sdist. */ +#include "c-ctype.h" #include "ignore-value.h" /* NB: MUST NOT include "guestfs-internal.h". */ @@ -360,6 +363,49 @@ guestfs_int_is_true (const char *str) return -1; } +/** + * Check a string for validity, that it contains only certain + * characters, and minimum and maximum length. This function is + * usually wrapped in a VALID_* macro, see F<src/drives.c> for an + * example. + * + * C<str> is the string to check. + * + * C<min_length> and C<max_length> are the minimum and maximum + * length checks. + * + * C<alpha> means 7-bit ASCII-only alphabetic characters are + * permitted. C<digit> means 7-bit ASCII-only digits are permitted. + * C<extra> is a set of extra characters permitted. + * + * Returns boolean C<true> if the string is valid (passes all the + * tests), or C<false> if not. + */ +bool +guestfs_int_string_is_valid (const char *str, + size_t min_length, size_t max_length, + bool alpha, bool digit, const char *extra) +{ + size_t i, len = strlen (str); + + if ((min_length > 0 && len < min_length) || + (max_length < SIZE_MAX && len > max_length)) + return false; + + for (i = 0; i < len; ++i) { + bool valid_char; + + valid_char = + (alpha && c_isalpha (str[i])) || + (digit && c_isdigit (str[i])) || + (extra && strchr (extra, str[i])); + + if (!valid_char) return false; + } + + return true; +} + #if 0 /* not used yet */ /** * Hint that we will read or write the file descriptor normally. -- 2.9.3