Bug 499791

Summary: virt-manager does not escape special characters in ISO file name
Product: [Fedora] Fedora Reporter: Ralf Ertzinger <redhat-bugzilla>
Component: libvirtAssignee: Daniel Veillard <veillard>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: berrange, clalance, crobinso, hbrock, itamar, markmc, quintela, veillard, virt-maint
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: 0.6.2-11.fc11 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-06-04 17:21:40 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Bug Depends On:    
Bug Blocks: 480594    
Attachments:
Description Flags
virt-manager.log excerpt
none
Fedora_11.xml
none
Patch to escape apos and quot none

Description Ralf Ertzinger 2009-05-08 03:25:51 EDT
Description of problem:
When attaching an ISO file as CDROM device in virt-manager, special characters in the filename are not escaped before being written into the XML file. Especially the "'" character is not escaped, breaking the XML file syntax and rendering the VM inaccessible until the XML file is fixed.

Version-Release number of selected component (if applicable):
virt-manager-0.7.0-4.fc11.i586

How reproducible:
Always

Steps to Reproduce:
1. Add an ISO image as CDROM to a VM where the filename/path contains an "'" character
2.
3.
  
Actual results:
Backtrace in virt-manager, VM becomes inaccessible

Expected results:
Normal operation

Additional info:
Comment 1 Daniel Berrange 2009-05-08 05:43:15 EDT
Hmm, I could have sworn virt-manager already did XML escaping of these paths. Can you attach the backtrace, and $HOME/.virt-manager/virt-manager.log file which should show us the XML it has generated. It could be that libvirt is not handling the special characters properly later in the sequence
Comment 2 Ralf Ertzinger 2009-05-08 06:07:46 EDT
I'll attach the relevant lines from the virt-manager.log file. Stragely enough I did not get a backtrace window on attaching the image, but the log file contains the backtrace.

The iso was named foo'bar.iso
Comment 3 Ralf Ertzinger 2009-05-08 06:10:08 EDT
Created attachment 343066 [details]
virt-manager.log excerpt
Comment 4 Ralf Ertzinger 2009-05-08 06:13:39 EDT
I noticed that the log file uses double quotes for the XML values, while the XML file on the disk uses single quotes.
Comment 5 Daniel Berrange 2009-05-08 06:28:24 EDT
The virt-manager generated XML looks ok to me, can you attach the output of 'virsh dumpxml Fedora_11' too
Comment 6 Ralf Ertzinger 2009-05-08 07:36:08 EDT
Created attachment 343072 [details]
Fedora_11.xml
Comment 7 Daniel Berrange 2009-05-08 07:42:44 EDT
Ok, this is libvirt's fault. We call virBufferEscapeString() when printing the <source file='XXX'> element, but the  function only escapes the characters neccessary for safe element content. It does not escape the characters neccessary for safe attribute content.  

We need to add another virBufferEscapeAttrString(), which does the same, but also escapes " and ', and then use it where neccessary.
Comment 8 Daniel Veillard 2009-05-12 11:05:06 EDT
Yes and no, basically ' or " are available as-is in attribute values,
they don't have to be escaped, they just need to be escaped if
both of them are present. Libxml2 serializer does that but well
we don't use it in libvirt. So yeah in that case the simpler is probably
to systematically escape ' and " in all content, attribute or not,
using &quot; or &apos; in non attribute text content is fine too
even if not needed, that should not generate any trouble unless someone
uses a non XML tool to process the XML files.

Daniel
Comment 9 Daniel Veillard 2009-05-12 11:09:54 EDT
Created attachment 343593 [details]
Patch to escape apos and quot
Comment 10 Fedora Update System 2009-05-21 09:25:15 EDT
libvirt-0.6.2-9.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/libvirt-0.6.2-9.fc11
Comment 11 Mark McLoughlin 2009-05-21 09:31:39 EDT
I've pushed libvirt-0.6.2-9.fc11 to updates-testing with DV's fix. Please test and update the update's karma using the link above

* Thu May 21 2009 Mark McLoughlin <markmc@redhat.com> - 0.6.2-9.fc11
- Fix qemu argv detection with latest qemu (bug #501923)
- Fix XML attribute escaping (bug #499791)
- Fix serious event handling issues causing guests to be destroyed (bug #499698)
Comment 12 Fedora Update System 2009-05-21 19:28:05 EDT
libvirt-0.6.2-9.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update libvirt'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-5311
Comment 13 Fedora Update System 2009-05-22 05:53:23 EDT
libvirt-0.6.2-10.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/libvirt-0.6.2-10.fc11
Comment 14 Fedora Update System 2009-05-25 17:20:08 EDT
libvirt-0.6.2-10.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update libvirt'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-5441
Comment 15 Fedora Update System 2009-05-26 03:57:46 EDT
libvirt-0.6.2-11.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update libvirt'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-5515
Comment 16 Fedora Update System 2009-06-04 17:21:13 EDT
libvirt-0.6.2-11.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 17 Fedora Update System 2009-06-05 09:24:03 EDT
libvirt-0.6.2-12.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/libvirt-0.6.2-12.fc11