Skip to content

Commit

Permalink
fix CVE-2020-14940
Browse files Browse the repository at this point in the history 
see:
http://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2020-14940
https://sourceforge.net/p/tuxguitar/bugs/126/
https://bugzilla.opensuse.org/show_bug.cgi?id=1173633
https://logicaltrust.net/blog/2020/06/tuxguitar.html
https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html

Issue could be reproduced on Linux before the fix, as described by
sourceforge page listed above
note: needed to de-activate firewall to reproduce issue
Not all TuxGuitar files mentioned in this page have been modified, as some
of them do not parse input xml files. Then they should not be concerned by
vulnerability:
- TuxGuitar-musicxml/src/org/herac/tuxguitar/io/musicxml/MusicXMLWriter.java
- TuxGuitar/src/org/herac/tuxguitar/app/system/keybindings/xml/KeyBindingWriter.java
- TuxGuitar/src/org/herac/tuxguitar/app/tools/browser/xml/TGBrowserWriter.java

note: protection does not seem to be supported on Android, so just try to
activate it, and ignore if it fails (or else Android version can no more
open .gp and .gpx files)
basically: this patch doesn't provide full coverage

also (independent from CVE):
GPXDocumentReader: removed warnings from deprecated Integer constructors
  • Loading branch information
@guiv42
guiv42 committed Oct 10, 2023
1 parent 8940135 commit bcaa280
Show file tree
Hide file tree
Showing 8 changed files with 71 additions and 17 deletions.
6 changes: 6 additions & 0 deletions TuxGuitar-community/src/org/herac/tuxguitar/community/browser/TGBrowserResponse.java
View file
Expand Up @@ -35,6 +35,12 @@ public TGBrowserResponse( InputStream stream ) throws Throwable {

private void initialize(InputStream stream) throws Throwable {
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
// CVE-2020-14940
try {
factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
factory.setXIncludeAware(false);
} catch (Throwable throwable) {
}
DocumentBuilder builder = factory.newDocumentBuilder();
this.document = builder.parse(stream);
}
Expand Down
6 changes: 6 additions & 0 deletions TuxGuitar-community/src/org/herac/tuxguitar/community/io/TGShareSongResponse.java
View file
Expand Up @@ -28,6 +28,12 @@ public TGShareSongResponse( InputStream stream ) throws Throwable {

private void initialize(InputStream stream) throws Throwable {
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
// CVE-2020-14940
try {
factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
factory.setXIncludeAware(false);
} catch (Throwable throwable) {
}
DocumentBuilder builder = factory.newDocumentBuilder();
this.document = builder.parse(stream);
}
Expand Down
6 changes: 6 additions & 0 deletions TuxGuitar-editor-utils/src/org/herac/tuxguitar/editor/template/TGTemplateReader.java
View file
Expand Up @@ -51,6 +51,12 @@ private void loadTemplates(List<TGTemplate> templates,Node node) throws Throwabl
private Document createDocument(InputStream stream) throws Throwable {
Document document = null;
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
// CVE-2020-14940
try {
factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
factory.setXIncludeAware(false);
} catch (Throwable throwable) {
}

DocumentBuilder builder = factory.newDocumentBuilder();
document = builder.parse(stream);
Expand Down
38 changes: 23 additions & 15 deletions TuxGuitar-gpx/src/org/herac/tuxguitar/io/gpx/GPXDocumentReader.java
View file
Expand Up @@ -35,8 +35,16 @@ public GPXDocumentReader(InputStream stream, Integer version) throws GPXFormatEx
}

private Document getDocument(InputStream stream) {
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
// CVE-2020-14940
try {
return DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(stream);
factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
factory.setXIncludeAware(false);
} catch (Throwable throwable) {
}

try {
return factory.newDocumentBuilder().parse(stream);
} catch (Throwable throwable) {
throw new GPXFormatException("Invalid file format", throwable);
}
Expand Down Expand Up @@ -309,25 +317,25 @@ public void readBeats(){
beat.setWhammyBarEnabled( getChildNode(propertyNode, "Enable") != null );
}
if( propertyName.equals("WhammyBarOriginValue") ){
beat.setWhammyBarOriginValue( new Integer(getChildNodeIntegerContent(propertyNode, "Float")) );
beat.setWhammyBarOriginValue( Integer.valueOf(getChildNodeIntegerContent(propertyNode, "Float")) );
}
if( propertyName.equals("WhammyBarMiddleValue") ){
beat.setWhammyBarMiddleValue( new Integer(getChildNodeIntegerContent(propertyNode, "Float")) );
beat.setWhammyBarMiddleValue( Integer.valueOf(getChildNodeIntegerContent(propertyNode, "Float")) );
}
if( propertyName.equals("WhammyBarDestinationValue") ){
beat.setWhammyBarDestinationValue( new Integer(getChildNodeIntegerContent(propertyNode, "Float")) );
beat.setWhammyBarDestinationValue( Integer.valueOf(getChildNodeIntegerContent(propertyNode, "Float")) );
}
if( propertyName.equals("WhammyBarOriginOffset") ){
beat.setWhammyBarOriginOffset( new Integer(getChildNodeIntegerContent(propertyNode, "Float")) );
beat.setWhammyBarOriginOffset( Integer.valueOf(getChildNodeIntegerContent(propertyNode, "Float")) );
}
if( propertyName.equals("WhammyBarMiddleOffset1") ){
beat.setWhammyBarMiddleOffset1( new Integer(getChildNodeIntegerContent(propertyNode, "Float")) );
beat.setWhammyBarMiddleOffset1( Integer.valueOf(getChildNodeIntegerContent(propertyNode, "Float")) );
}
if( propertyName.equals("WhammyBarMiddleOffset2") ){
beat.setWhammyBarMiddleOffset2( new Integer(getChildNodeIntegerContent(propertyNode, "Float")) );
beat.setWhammyBarMiddleOffset2( Integer.valueOf(getChildNodeIntegerContent(propertyNode, "Float")) );
}
if( propertyName.equals("WhammyBarDestinationOffset") ){
beat.setWhammyBarDestinationOffset( new Integer(getChildNodeIntegerContent(propertyNode, "Float")) );
beat.setWhammyBarDestinationOffset( Integer.valueOf(getChildNodeIntegerContent(propertyNode, "Float")) );
}
if( propertyName.equals("Brush") ){
beat.setBrush( getChildNodeContent(propertyNode, "Direction") );
Expand Down Expand Up @@ -408,25 +416,25 @@ public void readNotes(){
note.setBendEnabled( getChildNode(propertyNode, "Enable") != null );
}
if( propertyName.equals("BendOriginValue") ){
note.setBendOriginValue( new Integer(getChildNodeIntegerContent(propertyNode, "Float")) );
note.setBendOriginValue( Integer.valueOf(getChildNodeIntegerContent(propertyNode, "Float")) );
}
if( propertyName.equals("BendMiddleValue") ){
note.setBendMiddleValue( new Integer(getChildNodeIntegerContent(propertyNode, "Float")) );
note.setBendMiddleValue( Integer.valueOf(getChildNodeIntegerContent(propertyNode, "Float")) );
}
if( propertyName.equals("BendDestinationValue") ){
note.setBendDestinationValue( new Integer(getChildNodeIntegerContent(propertyNode, "Float")) );
note.setBendDestinationValue( Integer.valueOf(getChildNodeIntegerContent(propertyNode, "Float")) );
}
if( propertyName.equals("BendOriginOffset") ){
note.setBendOriginOffset( new Integer(getChildNodeIntegerContent(propertyNode, "Float")) );
note.setBendOriginOffset( Integer.valueOf(getChildNodeIntegerContent(propertyNode, "Float")) );
}
if( propertyName.equals("BendMiddleOffset1") ){
note.setBendMiddleOffset1( new Integer(getChildNodeIntegerContent(propertyNode, "Float")) );
note.setBendMiddleOffset1( Integer.valueOf(getChildNodeIntegerContent(propertyNode, "Float")) );
}
if( propertyName.equals("BendMiddleOffset2") ){
note.setBendMiddleOffset2( new Integer(getChildNodeIntegerContent(propertyNode, "Float")) );
note.setBendMiddleOffset2( Integer.valueOf(getChildNodeIntegerContent(propertyNode, "Float")) );
}
if( propertyName.equals("BendDestinationOffset") ){
note.setBendDestinationOffset( new Integer(getChildNodeIntegerContent(propertyNode, "Float")) );
note.setBendDestinationOffset( Integer.valueOf(getChildNodeIntegerContent(propertyNode, "Float")) );
}
if( propertyName.equals("HopoOrigin") ){
note.setHammer(true);
Expand Down
12 changes: 12 additions & 0 deletions TuxGuitar/src/org/herac/tuxguitar/app/system/keybindings/xml/KeyBindingReader.java
View file
Expand Up @@ -52,6 +52,12 @@ public static List<KeyBindingAction> getKeyBindings(InputStream is) {
private static Document getDocument(InputStream is) {
Document document = null;
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
// CVE-2020-14940
try {
factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
factory.setXIncludeAware(false);
} catch (Throwable throwable) {
}
try {
DocumentBuilder builder = factory.newDocumentBuilder();
document = builder.parse(is);
Expand All @@ -69,6 +75,12 @@ private static Document getDocument(InputStream is) {
private static Document getDocument(File file) {
Document document = null;
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
// CVE-2020-14940
try {
factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
factory.setXIncludeAware(false);
} catch (Throwable throwable) {
}
try {
DocumentBuilder builder = factory.newDocumentBuilder();
document = builder.parse(file);
Expand Down
7 changes: 6 additions & 1 deletion TuxGuitar/src/org/herac/tuxguitar/app/tools/browser/xml/TGBrowserReader.java
View file
Expand Up @@ -59,7 +59,12 @@ private static void loadCollections(TGBrowserManager manager,Node node){
private static Document getDocument(File file) throws ParserConfigurationException, SAXException, IOException {
Document document = null;
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();

// CVE-2020-14940
try {
factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
factory.setXIncludeAware(false);
} catch (Throwable throwable) {
}
DocumentBuilder builder = factory.newDocumentBuilder();
document = builder.parse(file);

Expand Down
7 changes: 6 additions & 1 deletion TuxGuitar/src/org/herac/tuxguitar/app/tools/scale/xml/ScaleReader.java
View file
Expand Up @@ -34,7 +34,12 @@ public void loadScales(List<ScaleInfo> scales,InputStream stream){
private static Document getDocument(InputStream stream) throws ParserConfigurationException, SAXException, IOException {
Document document = null;
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();

// CVE-2020-14940
try {
factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
factory.setXIncludeAware(false);
} catch (Throwable throwable) {
}
DocumentBuilder builder = factory.newDocumentBuilder();
document = builder.parse(stream);

Expand Down
6 changes: 6 additions & 0 deletions TuxGuitar/src/org/herac/tuxguitar/app/view/dialog/chord/xml/TGChordXMLReader.java
View file
Expand Up @@ -36,6 +36,12 @@ public static List<TGChord> getChords(String fileName) {
private static Document getDocument(File file) {
Document document = null;
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
// CVE-2020-14940
try {
factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
factory.setXIncludeAware(false);
} catch (Throwable throwable) {
}
try {
DocumentBuilder builder = factory.newDocumentBuilder();
document = builder.parse(file);
Expand Down

0 comments on commit bcaa280

Please sign in to comment.