Richard W.M. Jones wrote:
> Jim Paris wrote:
> >+ if (strchr(path, '\'') || strchr(path, '\\') ) {
> >+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
> >+ "invalid filename");
> >+ return -1;
> >+ }
> [...]
> >+ /* Migrate to file. */
> >+ if (asprintf (&command, "migrate \"exec:dd of='%s' 2>/dev/null\"\n",
> >+ path) == -1) {
> >+ qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
> >+ "out of memory");
> >+ return -1;
> >+ }
>
> The patch is fine, except I'm wondering whether the quoting above is
> safe. We check if the path contains ' or \ and refuse to proceed. I
> _think_ you don't need to check for \ however
I think you're right. An even better fix would be to explicitly
escape bad characters in the path before passing them along. Giving
an error on the filename "Jim's VM" as it would do right now isn't
ideal.
-jim
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list