From 8d48e0ec97122d72a5ab224c535e51de6a2bef77 Mon Sep 17 00:00:00 2001 From: Simon Rettberg Date: Mon, 14 Feb 2022 12:22:48 +0100 Subject: Always pass charset in String constructor; don't treat strings as binary safe --- .../java/org/openslx/libvirt/domain/device/Video.java | 8 ++++---- .../org/openslx/libvirt/xml/LibvirtXmlDocument.java | 8 +++++--- .../openslx/libvirt/xml/LibvirtXmlSchemaValidator.java | 3 ++- src/main/java/org/openslx/util/ThriftUtil.java | 1 + .../configuration/VirtualizationConfigurationQemu.java | 4 ++-- .../VirtualizationConfigurationQemuUtils.java | 2 +- ...irtualizationConfigurationVirtualboxFileFormat.java | 2 +- .../VirtualizationConfigurationVmwareFileFormat.java | 18 +++++++++++++++++- .../transformation/TransformationManager.java | 6 +++--- .../org/openslx/virtualization/disk/DiskImage.java | 2 +- .../openslx/virtualization/disk/DiskImageQcow2.java | 2 +- .../openslx/virtualization/disk/DiskImageUtils.java | 8 ++++---- .../org/openslx/virtualization/disk/DiskImageVdi.java | 15 +++++++++++---- .../org/openslx/virtualization/disk/DiskImageVmdk.java | 7 ++++--- 14 files changed, 57 insertions(+), 29 deletions(-) diff --git a/src/main/java/org/openslx/libvirt/domain/device/Video.java b/src/main/java/org/openslx/libvirt/domain/device/Video.java index f3ee13f..a674715 100644 --- a/src/main/java/org/openslx/libvirt/domain/device/Video.java +++ b/src/main/java/org/openslx/libvirt/domain/device/Video.java @@ -73,8 +73,8 @@ public class Video extends Device // only set acceleration on supported Virtio GPUs this.setXmlElementAttributeValueYesNo( "model/acceleration", "accel2d", acceleration ); } else { - String errorMsg = new String( - "Video card model '" + model.toString() + "' does not support enabled 2D hardware acceleration." ); + String errorMsg = + "Video card model '" + model.toString() + "' does not support enabled 2D hardware acceleration."; throw new IllegalArgumentException( errorMsg ); } } @@ -103,8 +103,8 @@ public class Video extends Device // only set acceleration on supported Virtio GPUs this.setXmlElementAttributeValueYesNo( "model/acceleration", "accel3d", acceleration ); } else { - String errorMsg = new String( - "Video card model '" + model.toString() + "' does not support enabled 3D hardware acceleration." ); + String errorMsg = + "Video card model '" + model.toString() + "' does not support enabled 3D hardware acceleration."; throw new IllegalArgumentException( errorMsg ); } } diff --git a/src/main/java/org/openslx/libvirt/xml/LibvirtXmlDocument.java b/src/main/java/org/openslx/libvirt/xml/LibvirtXmlDocument.java index 97180e3..c428988 100644 --- a/src/main/java/org/openslx/libvirt/xml/LibvirtXmlDocument.java +++ b/src/main/java/org/openslx/libvirt/xml/LibvirtXmlDocument.java @@ -10,6 +10,7 @@ import java.io.StringWriter; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; +import javax.xml.transform.OutputKeys; import javax.xml.transform.Transformer; import javax.xml.transform.TransformerConfigurationException; import javax.xml.transform.TransformerException; @@ -74,7 +75,7 @@ public abstract class LibvirtXmlDocument implements LibvirtXmlSerializable, Libv domFactory.setNamespaceAware( true ); this.domBuilder = domFactory.newDocumentBuilder(); } catch ( ParserConfigurationException e ) { - String errorMsg = new String( "Setting up XML context for reading from the Libvirt XML document failed." ); + String errorMsg = "Setting up XML context for reading from the Libvirt XML document failed."; throw new LibvirtXmlDocumentException( errorMsg ); } @@ -91,8 +92,9 @@ public abstract class LibvirtXmlDocument implements LibvirtXmlSerializable, Libv InputStream xslOutputSchemaStream = LibvirtXmlResources.getLibvirtXsl( "xml-output-transformation.xsl" ); StreamSource xslOutputSchema = new StreamSource( xslOutputSchemaStream ); this.xmlTransformer = transformerFactory.newTransformer( xslOutputSchema ); + this.xmlTransformer.setOutputProperty( OutputKeys.ENCODING, "UTF-8" ); } catch ( TransformerConfigurationException e ) { - String errorMsg = new String( "Setting up XML context for writing to the Libvirt XML document failed." ); + String errorMsg = "Setting up XML context for writing to the Libvirt XML document failed."; throw new LibvirtXmlDocumentException( errorMsg ); } @@ -101,7 +103,7 @@ public abstract class LibvirtXmlDocument implements LibvirtXmlSerializable, Libv try { this.rngValidator = new LibvirtXmlSchemaValidator( rngSchema ); } catch ( SAXException e ) { - String errorMsg = new String( "Setting up XML context for validating to the Libvirt XML document failed." ); + String errorMsg = "Setting up XML context for validating to the Libvirt XML document failed."; e.printStackTrace(); throw new LibvirtXmlDocumentException( errorMsg ); } diff --git a/src/main/java/org/openslx/libvirt/xml/LibvirtXmlSchemaValidator.java b/src/main/java/org/openslx/libvirt/xml/LibvirtXmlSchemaValidator.java index bc8f90f..d589d41 100644 --- a/src/main/java/org/openslx/libvirt/xml/LibvirtXmlSchemaValidator.java +++ b/src/main/java/org/openslx/libvirt/xml/LibvirtXmlSchemaValidator.java @@ -6,6 +6,7 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.Reader; +import java.nio.charset.StandardCharsets; import javax.xml.XMLConstants; import javax.xml.transform.Transformer; @@ -106,7 +107,7 @@ class LibvirtXmlSchemaResourceInput implements LSInput int inputLength = this.inputStream.available(); byte[] input = new byte[ inputLength ]; this.inputStream.read( input ); - data = new String( input ); + data = new String( input, StandardCharsets.UTF_8 ); } catch ( IOException e ) { e.printStackTrace(); } diff --git a/src/main/java/org/openslx/util/ThriftUtil.java b/src/main/java/org/openslx/util/ThriftUtil.java index a9035c1..ef10798 100644 --- a/src/main/java/org/openslx/util/ThriftUtil.java +++ b/src/main/java/org/openslx/util/ThriftUtil.java @@ -41,6 +41,7 @@ public class ThriftUtil { BufferedReader reader; StringBuffer content = new StringBuffer(""); try { + // Why is a generic function to convert a buffer to string CALLING A VMWARE SPECIFIC FUNCTION!? reader = VirtualizationConfigurationVmwareFileFormat.getVmxReader(bytes, bytes.length); String line=""; while ((line=reader.readLine()) != null) { diff --git a/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationQemu.java b/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationQemu.java index 7082e27..45b4f1f 100644 --- a/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationQemu.java +++ b/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationQemu.java @@ -115,7 +115,7 @@ public class VirtualizationConfigurationQemu extends VirtualizationConfiguration try { // read and parse Libvirt domain XML configuration document - this.vmConfig = new Domain( new String( vmContent ) ); + this.vmConfig = new Domain( new String( vmContent, StandardCharsets.UTF_8 ) ); } catch ( LibvirtXmlDocumentException | LibvirtXmlSerializationException | LibvirtXmlValidationException e ) { throw new VirtualizationConfigurationException( e.getLocalizedMessage() ); } @@ -239,7 +239,7 @@ public class VirtualizationConfigurationQemu extends VirtualizationConfiguration @Override public boolean addEmptyHddTemplate() { - return this.addHddTemplate( new String(), null, null ); + return this.addHddTemplate( "", null, null ); } @Override diff --git a/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationQemuUtils.java b/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationQemuUtils.java index 1befdc4..7b1b9bb 100644 --- a/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationQemuUtils.java +++ b/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationQemuUtils.java @@ -114,7 +114,7 @@ public class VirtualizationConfigurationQemuUtils private static String createAlphabeticalDeviceName( String devicePrefix, int deviceNumber ) { if ( deviceNumber < 0 || deviceNumber >= ( 'z' - 'a' ) ) { - String errorMsg = new String( "Device number is out of range to be able to create a valid device name." ); + String errorMsg = "Device number is out of range to be able to create a valid device name."; throw new IllegalArgumentException( errorMsg ); } diff --git a/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationVirtualboxFileFormat.java b/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationVirtualboxFileFormat.java index b4cb7c3..af7c54c 100644 --- a/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationVirtualboxFileFormat.java +++ b/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationVirtualboxFileFormat.java @@ -44,7 +44,7 @@ public class VirtualizationConfigurationVirtualboxFileFormat private static final Logger LOGGER = LogManager.getLogger( VirtualizationConfigurationVirtualboxFileFormat.class ); // key information set during initial parsing of the XML file - private String osName = new String(); + private String osName = ""; private ArrayList hddsArray = new ArrayList(); // XPath and DOM parsing related members diff --git a/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationVmwareFileFormat.java b/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationVmwareFileFormat.java index 134ff30..356a034 100644 --- a/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationVmwareFileFormat.java +++ b/src/main/java/org/openslx/virtualization/configuration/VirtualizationConfigurationVmwareFileFormat.java @@ -117,12 +117,22 @@ public class VirtualizationConfigurationVmwareFileFormat public static BufferedReader getVmxReader( byte[] vmxContent, int length ) throws IOException { Charset cs = getCharset( vmxContent, length ); + if ( cs == null ) + cs = StandardCharsets.UTF_8; // YES BECAUSE THIS IS NOT VMX AND EVERYTHING IS SHIT return new BufferedReader( new InputStreamReader( new ByteArrayInputStream( vmxContent, 0, length ), cs ) ); } + /** + * Get charset of config. Returns null if input doesn't look like a vmx file. + * @param vmxContent + * @param length + * @return + */ public static Charset getCharset( byte[] vmxContent, int length ) { String csName = detectCharset( new ByteArrayInputStream( vmxContent, 0, length ) ); + if ( csName == null ) + return null; Charset cs = null; try { cs = Charset.forName( csName ); @@ -146,8 +156,9 @@ public class VirtualizationConfigurationVmwareFileFormat return ret; } - public static String detectCharset( InputStream is ) + private static String detectCharset( InputStream is ) { + boolean isVmware = false; try { BufferedReader csDetectReader = new BufferedReader( new InputStreamReader( is, StandardCharsets.ISO_8859_1 ) ); String line; @@ -158,10 +169,15 @@ public class VirtualizationConfigurationVmwareFileFormat if ( entry.key.equals( ".encoding" ) || entry.key.equals( "encoding" ) ) { return entry.value; } + if ( entry.key.equals( "virtualHW.version" ) || entry.key.equals( "memsize" ) || entry.key.equals( "displayName") ) { + isVmware = true; + } } } catch ( Exception e ) { LOGGER.warn( "Could not detect charset, fallback to latin1", e ); } + if ( !isVmware ) + return null; // Dumb fallback return "ISO-8859-1"; } diff --git a/src/main/java/org/openslx/virtualization/configuration/transformation/TransformationManager.java b/src/main/java/org/openslx/virtualization/configuration/transformation/TransformationManager.java index 6bb5faa..18d7e43 100644 --- a/src/main/java/org/openslx/virtualization/configuration/transformation/TransformationManager.java +++ b/src/main/java/org/openslx/virtualization/configuration/transformation/TransformationManager.java @@ -122,8 +122,8 @@ public class TransformationManager try { transformation.apply( this.config, this.args ); } catch ( TransformationException e ) { - final String errorMsg = new String( - "Error in configuration filter '" + transformation.getName() + "': " + e.getLocalizedMessage() ); + final String errorMsg = + "Error in configuration filter '" + transformation.getName() + "': " + e.getLocalizedMessage(); throw new TransformationException( errorMsg ); } } @@ -136,7 +136,7 @@ public class TransformationManager */ private String showTransformations() { - String transformationSummary = new String(); + String transformationSummary = ""; final int maxFilterNumCharacters = ( this.transformations.size() + 1 ) / 10; for ( int i = 0; i < this.transformations.size(); i++ ) { diff --git a/src/main/java/org/openslx/virtualization/disk/DiskImage.java b/src/main/java/org/openslx/virtualization/disk/DiskImage.java index 5271e6e..dfb242a 100644 --- a/src/main/java/org/openslx/virtualization/disk/DiskImage.java +++ b/src/main/java/org/openslx/virtualization/disk/DiskImage.java @@ -136,7 +136,7 @@ public abstract class DiskImage implements Closeable throw e; } Util.safeClose( fileHandle ); - final String errorMsg = new String( "File '" + diskImagePath.getAbsolutePath() + "' is not a valid disk image!" ); + final String errorMsg = "File '" + diskImagePath.getAbsolutePath() + "' is not a valid disk image!"; throw new DiskImageException( errorMsg ); } diff --git a/src/main/java/org/openslx/virtualization/disk/DiskImageQcow2.java b/src/main/java/org/openslx/virtualization/disk/DiskImageQcow2.java index a007e1c..a6b3b73 100644 --- a/src/main/java/org/openslx/virtualization/disk/DiskImageQcow2.java +++ b/src/main/java/org/openslx/virtualization/disk/DiskImageQcow2.java @@ -210,7 +210,7 @@ public class DiskImageQcow2 extends DiskImage // check QCOW2 file format version if ( qcowVersion < 2 || qcowVersion > 3 ) { // QCOW2 disk image does not contain a valid QCOW2 version - final String errorMsg = new String( "Invalid QCOW2 version in header found!" ); + final String errorMsg = "Invalid QCOW2 version in header found!"; throw new DiskImageException( errorMsg ); } diff --git a/src/main/java/org/openslx/virtualization/disk/DiskImageUtils.java b/src/main/java/org/openslx/virtualization/disk/DiskImageUtils.java index dc67548..6bcef7f 100644 --- a/src/main/java/org/openslx/virtualization/disk/DiskImageUtils.java +++ b/src/main/java/org/openslx/virtualization/disk/DiskImageUtils.java @@ -119,11 +119,11 @@ public class DiskImageUtils * @param diskImage file to a disk storing the image content. * @param offset offset in bytes for reading numBytes bytes. * @param numBytes number of bytes to read at offset. - * @return read bytes from the disk image file as {@link String}. + * @return read bytes from the disk image file as {@link byte[]}. * - * @throws DiskImageException unable to read two bytes from the disk image file. + * @throws DiskImageException unable to read bytes from the disk image file. */ - public static String readBytesAsString( RandomAccessFile diskImage, long offset, int numBytes ) + public static byte[] readBytesAsArray( RandomAccessFile diskImage, long offset, int numBytes ) throws DiskImageException { final long imageSize = DiskImageUtils.getImageSize( diskImage ); @@ -139,6 +139,6 @@ public class DiskImageUtils } } - return new String( values ); + return values; } } diff --git a/src/main/java/org/openslx/virtualization/disk/DiskImageVdi.java b/src/main/java/org/openslx/virtualization/disk/DiskImageVdi.java index c564112..1e5b20b 100644 --- a/src/main/java/org/openslx/virtualization/disk/DiskImageVdi.java +++ b/src/main/java/org/openslx/virtualization/disk/DiskImageVdi.java @@ -1,6 +1,8 @@ package org.openslx.virtualization.disk; import java.io.RandomAccessFile; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; import org.openslx.virtualization.Version; @@ -16,6 +18,10 @@ public class DiskImageVdi extends DiskImage * Big endian representation of the little endian VDI magic bytes (signature). */ private static final int VDI_MAGIC = 0x7f10dabe; + /** + * Just 16 null-bytes + */ + private static final byte[] ZERO_UUID = new byte[16]; /** * Creates a new VDI disk image from an existing VDI image file. @@ -73,10 +79,9 @@ public class DiskImageVdi extends DiskImage final RandomAccessFile diskFile = this.getDiskImage(); // if parent UUID is set, the VDI file is a snapshot - final String parentUuid = DiskImageUtils.readBytesAsString( diskFile, 440, 16 ); - final String zeroUuid = new String( new byte[ 16 ] ); + byte[] parentUuid = DiskImageUtils.readBytesAsArray( diskFile, 440, 16 ); - return !zeroUuid.equals( parentUuid ); + return !Arrays.equals( ZERO_UUID, parentUuid ); } @Override @@ -94,7 +99,9 @@ public class DiskImageVdi extends DiskImage public String getDescription() throws DiskImageException { final RandomAccessFile diskFile = this.getDiskImage(); - return DiskImageUtils.readBytesAsString( diskFile, 84, 256 ); + byte[] data = DiskImageUtils.readBytesAsArray( diskFile, 84, 256 ); + // This will replace invalid chars. Maybe use CharsetDecoder and fall back to latin1 on error + return new String( data, StandardCharsets.UTF_8 ); } @Override diff --git a/src/main/java/org/openslx/virtualization/disk/DiskImageVmdk.java b/src/main/java/org/openslx/virtualization/disk/DiskImageVmdk.java index 720ec37..77986ef 100644 --- a/src/main/java/org/openslx/virtualization/disk/DiskImageVmdk.java +++ b/src/main/java/org/openslx/virtualization/disk/DiskImageVmdk.java @@ -1,6 +1,7 @@ package org.openslx.virtualization.disk; import java.io.RandomAccessFile; +import java.nio.charset.StandardCharsets; import org.openslx.util.Util; import org.openslx.virtualization.configuration.VirtualizationConfigurationVmwareFileFormat; @@ -117,15 +118,15 @@ public class DiskImageVmdk extends DiskImage // get content of descriptor file embedded into the VMDK disk image final long vmdkDescriptorOffset = vmdkDescriptorSectorOffset * DiskImageVmdk.VMDK_SECTOR_SIZE; final long vmdkDescriptorSizeMax = vmdkDescriptorSectorSize * DiskImageVmdk.VMDK_SECTOR_SIZE; - final String descriptorStr = DiskImageUtils.readBytesAsString( diskFile, vmdkDescriptorOffset, - Long.valueOf( vmdkDescriptorSizeMax ).intValue() ); + final String descriptorStr = new String ( DiskImageUtils.readBytesAsArray( diskFile, vmdkDescriptorOffset, + Long.valueOf( vmdkDescriptorSizeMax ).intValue() ), StandardCharsets.US_ASCII ); // get final length of the content within the sectors to be able to trim all 'zero' characters final int vmdkDescriptorSize = descriptorStr.indexOf( 0 ); // if final length of the content is invalid, throw an exception if ( vmdkDescriptorSize > vmdkDescriptorSizeMax || vmdkDescriptorSize < 0 ) { - final String errorMsg = new String( "Embedded descriptor size in VMDK disk image is invalid!" ); + final String errorMsg = "Embedded descriptor size in VMDK disk image is invalid!"; throw new DiskImageException( errorMsg ); } -- cgit v1.2.3-55-g7522