Skip to content

Commit

Permalink
added a new read config flag of anchor to enable/disable following an… (
Browse files Browse the repository at this point in the history
#164)

* added a new read config flag of anchor to enable/disable following anchors ( enabled by default ).
Also added a new SafeYamlConfig class that disables Class Tags and Anchors to remediate
CVE-2023-24620
CVE-2023-24621

* setAnchor() and setClassTag() now throws an IllegalArgumentException if an attempt is made to set them to true.
  • Loading branch information
JoeBeeton authored Feb 1, 2023
1 parent 08f4f7b commit b112258
Show file tree
Hide file tree
Showing 4 changed files with 156 additions and 9 deletions.
50 changes: 50 additions & 0 deletions src/com/esotericsoftware/yamlbeans/SafeYamlConfig.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package com.esotericsoftware.yamlbeans;


/**
* SafeYamlConfig extends YamlConfig and hard codes the read anchor and read class tag flags to false.
* When these flags are enabled, it is possible to perform a deserialization attack if the Yaml being parsed is from an
* untrusted source.
* Using SafeYamlConfig is the equivalent of using YamlConfig after setting
* yamlConfig.readConfig.setAnchors(false);
* yamlConfig.readConfig.setClassTags(false);
*
* It should be noted by setting these two values neither anchors or specifying class names are supported.
* It is still possible to deserialize back to a specific object, but you need to specify the Class type in the code.
* e.g
* SafeYamlConfig yamlConfig = new SafeYamlConfig();
* YamlReader reader = new YamlReader(yamlData.toString(),yamlConfig);
* Data data = reader.read(Data.class);
*
*/
public class SafeYamlConfig extends YamlConfig {


public SafeYamlConfig () {
super();
super.readConfig = new SafeReadConfig();
}

static public class SafeReadConfig extends ReadConfig {

public SafeReadConfig(){
super.anchors = false;
super.classTags = false;
}

@Override
public void setClassTags(boolean classTags) {
if(classTags) {
throw new IllegalArgumentException("Class Tags cannot be enabled in SafeYamlConfig.");
}
}

@Override
public void setAnchors(boolean anchors) {
if(anchors) {
throw new IllegalArgumentException("Anchors cannot be enabled in SafeYamlConfig.");
}
}
}

}
11 changes: 10 additions & 1 deletion src/com/esotericsoftware/yamlbeans/YamlConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public class YamlConfig {
public final WriteConfig writeConfig = new WriteConfig();

/** Configuration for reading YAML. */
public final ReadConfig readConfig = new ReadConfig();
public ReadConfig readConfig = new ReadConfig();

final Map<String, String> classNameToTag = new HashMap();
final Map<String, Class> tagToClass = new HashMap();
Expand Down Expand Up @@ -268,6 +268,10 @@ static public class ReadConfig {
boolean classTags = true;
boolean guessNumberTypes;



boolean anchors = true;

ReadConfig () {
}

Expand Down Expand Up @@ -320,6 +324,11 @@ public void setAutoMerge (boolean autoMerge) {
public void setGuessNumberTypes (boolean guessNumberTypes) {
this.guessNumberTypes = guessNumberTypes;
}

public void setAnchors(boolean anchors) {
this.anchors = anchors;
}

}

static class ConstructorParameters {
Expand Down
25 changes: 17 additions & 8 deletions src/com/esotericsoftware/yamlbeans/YamlReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,13 @@ public Object get (String alias) {
return anchors.get(alias);
}

private void addAnchor(String key, Object value) {
if(config.readConfig.anchors) {
anchors.put(key, value);
}
}


public void close () throws IOException {
parser.close();
anchors.clear();
Expand Down Expand Up @@ -157,7 +164,7 @@ protected Object readValue (Class type, Class elementType, Class defaultType)
parser.getNextEvent();
anchor = ((AliasEvent)event).anchor;
Object value = anchors.get(anchor);
if (value == null) throw new YamlReaderException("Unknown anchor: " + anchor);
if (value == null&&config.readConfig.anchors) throw new YamlReaderException("Unknown anchor: " + anchor);
return value;
case MAPPING_START:
case SEQUENCE_START:
Expand Down Expand Up @@ -238,7 +245,7 @@ private Object readValueInternal (Class type, Class elementType, String anchor)
Number number = valueConvertedNumber(value);
if (number != null) {
if (anchor != null) {
anchors.put(anchor, number);
addAnchor(anchor, number);
}
parser.getNextEvent();
return number;
Expand Down Expand Up @@ -284,7 +291,9 @@ private Object readValueInternal (Class type, Class elementType, String anchor)
convertedValue = Byte.decode(value);
} else
throw new YamlException("Unknown field type.");
if (anchor != null) anchors.put(anchor, convertedValue);
if (anchor != null) {
addAnchor(anchor, convertedValue);
}
return convertedValue;
} catch (Exception ex) {
throw new YamlReaderException("Unable to convert value to required type \"" + type + "\": " + value, ex);
Expand All @@ -298,7 +307,7 @@ private Object readValueInternal (Class type, Class elementType, String anchor)
if (event.type != SCALAR) throw new YamlReaderException("Expected scalar for type '" + type
+ "' to be deserialized by scalar serializer '" + serializer.getClass().getName() + "' but found: " + event.type);
Object value = serializer.read(((ScalarEvent)event).value);
if (anchor != null) anchors.put(anchor, value);
if (anchor != null) addAnchor(anchor, value);
return value;
}
}
Expand Down Expand Up @@ -326,7 +335,7 @@ private Object readValueInternal (Class type, Class elementType, String anchor)
} catch (InvocationTargetException ex) {
throw new YamlReaderException("Error creating object.", ex);
}
if (anchor != null) anchors.put(anchor, object);
if (anchor != null) addAnchor(anchor, object);
ArrayList keys = new ArrayList();
while (true) {
if (parser.peekNextEvent().type == MAPPING_END) {
Expand Down Expand Up @@ -404,7 +413,7 @@ private Object readValueInternal (Class type, Class elementType, String anchor)
if (object instanceof DeferredConstruction) {
try {
object = ((DeferredConstruction)object).construct();
if (anchor != null) anchors.put(anchor, object); // Update anchor with real object.
if (anchor != null) addAnchor(anchor, object); // Update anchor with real object.
} catch (InvocationTargetException ex) {
throw new YamlReaderException("Error creating object.", ex);
}
Expand All @@ -426,7 +435,7 @@ private Object readValueInternal (Class type, Class elementType, String anchor)
elementType = type.getComponentType();
} else
throw new YamlReaderException("A sequence is not a valid value for the type: " + type.getName());
if (!type.isArray() && anchor != null) anchors.put(anchor, collection);
if (!type.isArray() && anchor != null) addAnchor(anchor, collection);
while (true) {
event = parser.peekNextEvent();
if (event.type == SEQUENCE_END) {
Expand All @@ -440,7 +449,7 @@ private Object readValueInternal (Class type, Class elementType, String anchor)
int i = 0;
for (Object object : collection)
Array.set(array, i++, object);
if (anchor != null) anchors.put(anchor, array);
if (anchor != null) addAnchor(anchor, array);
return array;
}
default:
Expand Down
79 changes: 79 additions & 0 deletions test/com/esotericsoftware/yamlbeans/SafeYamlConfigTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
package com.esotericsoftware.yamlbeans;

import org.junit.Test;

import java.util.HashMap;
import java.util.List;
import java.util.Map;

import static junit.framework.Assert.assertEquals;
import static junit.framework.Assert.assertNull;
import static junit.framework.Assert.assertTrue;

public class SafeYamlConfigTest {


private static final String TESTOBJECT_TAG = "!com.esotericsoftware.yamlbeans.SafeYamlConfigTest$TestObject";
private static final String LINE_SEPARATOR = System.getProperty("line.separator");


@Test
public void testDeserializationOfClassTag() throws YamlException {
SafeYamlConfig yamlConfig = new SafeYamlConfig();
StringBuilder yamlData = new StringBuilder();
yamlData.append(TESTOBJECT_TAG).append(LINE_SEPARATOR)
.append("a: test").append(LINE_SEPARATOR);
YamlReader reader = new YamlReader(yamlData.toString(),yamlConfig);
Object data = reader.read();
assertTrue(data instanceof HashMap);
Map dataMap = (Map) data;
assertTrue(dataMap.containsKey("a"));
assertEquals("test",dataMap.get("a"));
}



@Test
public void testIgnoreAnchor() throws YamlException {
SafeYamlConfig yamlConfig = new SafeYamlConfig();
StringBuilder yamlData = new StringBuilder();
yamlData.append("oldest friend:").append(LINE_SEPARATOR)
.append(" &1 !contact").append(LINE_SEPARATOR)
.append(" name: Bob").append(LINE_SEPARATOR)
.append(" age: 29").append(LINE_SEPARATOR)
.append("best friend: *1").append(LINE_SEPARATOR);
YamlReader reader = new YamlReader(yamlData.toString(),yamlConfig);
Object data = reader.read();
assertTrue(data instanceof HashMap);
Map dataMap = (Map) data;
assertTrue(dataMap.containsKey("oldest friend"));
Map old = (Map) dataMap.get("oldest friend");
assertTrue(old.containsKey("name"));
assertEquals("Bob",old.get("name"));
assertNull(dataMap.get("best friend"));
}


static class TestObject {
private String a;
public int age;
public String name;
public Object object;
public List<Object> objects;

private TestObject() {
}

public TestObject(String a) {
this.a = a;
}

public String getA() {
return a;
}

public void setA(String a) {
this.a = a;
}
}
}

0 comments on commit b112258

Please sign in to comment.