hazza/DSpace
1
0
Fork 0
mirror of https://github.com/DSpace/DSpace.git synced 2025-10-08 18:44:22 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #11075 from mwoodiupui/my-11042

Browse Source 
Avoid injection vulnerability in controlled vocabulary lookup
This commit is contained in:
Tim Donohue
2025-07-25 13:47:52 -05:00
committed by GitHub
parent 932f733f22 dac494191c
commit b51f579fc7
2 changed files with 69 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files

94
dspace-api/src/main/java/org/dspace/content/authority/DSpaceControlledVocabulary.java
View File

@@ -35,23 +35,25 @@ import org.xml.sax.InputSource;
* from {@code ${dspace.dir}/config/controlled-vocabularies/*.xml} and turns * from {@code ${dspace.dir}/config/controlled-vocabularies/*.xml} and turns
* them into autocompleting authorities. * them into autocompleting authorities.
* *
* Configuration: This MUST be configured as a self-named plugin, e.g.: {@code * <p>Configuration: This MUST be configured as a self-named plugin, e.g.: {@code
* plugin.selfnamed.org.dspace.content.authority.ChoiceAuthority = \ * plugin.selfnamed.org.dspace.content.authority.ChoiceAuthority =
* org.dspace.content.authority.DSpaceControlledVocabulary * org.dspace.content.authority.DSpaceControlledVocabulary
* } * }
* *
* It AUTOMATICALLY configures a plugin instance for each XML file in the * <p>It AUTOMATICALLY configures a plugin instance for each XML file in the
* controlled vocabularies directory. The name of the plugin is the basename of * controlled vocabularies directory. The name of the plugin is the basename of
* the file; e.g., {@code ${dspace.dir}/config/controlled-vocabularies/nsi.xml} * the file; e.g., {@code ${dspace.dir}/config/controlled-vocabularies/nsi.xml}
* would generate a plugin called "nsi". * would generate a plugin called "nsi".
* *
* Each configured plugin comes with three configuration options: {@code * <p>Each configured plugin comes with three configuration options:
* vocabulary.plugin._plugin_.hierarchy.store = <true|false> * <ul>
* # Store entire hierarchy along with selected value. Default: TRUE * <li>{@code vocabulary.plugin._plugin_.hierarchy.store = <true|false>
* vocabulary.plugin._plugin_.hierarchy.suggest = * # Store entire hierarchy along with selected value. Default: TRUE}</li>
* <true|false> # Display entire hierarchy in the suggestion list. Default: TRUE * <li>{@code vocabulary.plugin._plugin_.hierarchy.suggest =
* vocabulary.plugin._plugin_.delimiter = "<string>" * <true|false> # Display entire hierarchy in the suggestion list. Default: TRUE}</li>
* # Delimiter to use when building hierarchy strings. Default: "::" * <li>{@code vocabulary.plugin._plugin_.delimiter = "<string>"
* # Delimiter to use when building hierarchy strings. Default: "::"}</li>
* </ul>
* } * }
* *
* @author Michael B. Klein * @author Michael B. Klein
@@ -59,11 +61,12 @@ import org.xml.sax.InputSource;
public class DSpaceControlledVocabulary extends SelfNamedPlugin implements HierarchicalAuthority { public class DSpaceControlledVocabulary extends SelfNamedPlugin implements HierarchicalAuthority {
private static Logger log = org.apache.logging.log4j.LogManager.getLogger(DSpaceControlledVocabulary.class); private static final Logger log = org.apache.logging.log4j.LogManager.getLogger();
protected static String xpathTemplate = "//node[contains(translate(@label,'ABCDEFGHIJKLMNOPQRSTUVWXYZ'," + protected static String xpathTemplate = "//node[contains(translate(@label,'ABCDEFGHIJKLMNOPQRSTUVWXYZ'," +
"'abcdefghijklmnopqrstuvwxyz'),'%s')]"; "'abcdefghijklmnopqrstuvwxyz'),%s)]";
protected static String idTemplate = "//node[@id = '%s']"; protected static String idTemplate = "//node[@id = %s]";
protected static String labelTemplate = "//node[@label = '%s']"; protected static String idTemplateQuoted = "//node[@id = '%s']";
protected static String labelTemplate = "//node[@label = %s]";
protected static String idParentTemplate = "//node[@id = '%s']/parent::isComposedBy/parent::node"; protected static String idParentTemplate = "//node[@id = '%s']/parent::isComposedBy/parent::node";
protected static String rootTemplate = "/node"; protected static String rootTemplate = "/node";
protected static String idAttribute = "id"; protected static String idAttribute = "id";
@@ -110,7 +113,7 @@ public class DSpaceControlledVocabulary extends SelfNamedPlugin implements Hiera
File.separator + "config" + File.separator + "config" +
File.separator + "controlled-vocabularies"; File.separator + "controlled-vocabularies";
String[] xmlFiles = (new File(vocabulariesPath)).list(new xmlFilter()); String[] xmlFiles = (new File(vocabulariesPath)).list(new xmlFilter());
List<String> names = new ArrayList<String>(); List<String> names = new ArrayList<>();
for (String filename : xmlFiles) { for (String filename : xmlFiles) {
names.add((new File(filename)).getName().replace(".xml", "")); names.add((new File(filename)).getName().replace(".xml", ""));
} }
@@ -178,15 +181,23 @@ public class DSpaceControlledVocabulary extends SelfNamedPlugin implements Hiera
public Choices getMatches(String text, int start, int limit, String locale) { public Choices getMatches(String text, int start, int limit, String locale) {
init(locale); init(locale);
log.debug("Getting matches for '" + text + "'"); log.debug("Getting matches for '" + text + "'");
String xpathExpression = "";
String[] textHierarchy = text.split(hierarchyDelimiter, -1); String[] textHierarchy = text.split(hierarchyDelimiter, -1);
StringBuilder xpathExpressionBuilder = new StringBuilder();
for (int i = 0; i < textHierarchy.length; i++) { for (int i = 0; i < textHierarchy.length; i++) {
xpathExpression += xpathExpressionBuilder.append(String.format(xpathTemplate, "$var" + i));
String.format(xpathTemplate, textHierarchy[i].replaceAll("'", "&apos;").toLowerCase());
} }
String xpathExpression = xpathExpressionBuilder.toString();
XPath xpath = XPathFactory.newInstance().newXPath(); XPath xpath = XPathFactory.newInstance().newXPath();
int total = 0; xpath.setXPathVariableResolver(variableName -> {
List<Choice> choices = new ArrayList<Choice>(); String varName = variableName.getLocalPart();
if (varName.startsWith("var")) {
int index = Integer.parseInt(varName.substring(3));
return textHierarchy[index].toLowerCase();
}
throw new IllegalArgumentException("Unexpected variable: " + varName);
});
int total;
List<Choice> choices;
try { try {
NodeList results = (NodeList) xpath.evaluate(xpathExpression, vocabulary, XPathConstants.NODESET); NodeList results = (NodeList) xpath.evaluate(xpathExpression, vocabulary, XPathConstants.NODESET);
total = results.getLength(); total = results.getLength();
@@ -202,15 +213,23 @@ public class DSpaceControlledVocabulary extends SelfNamedPlugin implements Hiera
@Override @Override
public Choices getBestMatch(String text, String locale) { public Choices getBestMatch(String text, String locale) {
init(locale); init(locale);
log.debug("Getting best matches for '" + text + "'"); log.debug("Getting best matches for {}'", text);
String xpathExpression = "";
String[] textHierarchy = text.split(hierarchyDelimiter, -1); String[] textHierarchy = text.split(hierarchyDelimiter, -1);
StringBuilder xpathExpressionBuilder = new StringBuilder();
for (int i = 0; i < textHierarchy.length; i++) { for (int i = 0; i < textHierarchy.length; i++) {
xpathExpression += xpathExpressionBuilder.append(String.format(valueTemplate, "$var" + i));
String.format(valueTemplate, textHierarchy[i].replaceAll("'", "&apos;"));
} }
String xpathExpression = xpathExpressionBuilder.toString();
XPath xpath = XPathFactory.newInstance().newXPath(); XPath xpath = XPathFactory.newInstance().newXPath();
List<Choice> choices = new ArrayList<Choice>(); xpath.setXPathVariableResolver(variableName -> {
String varName = variableName.getLocalPart();
if (varName.startsWith("var")) {
int index = Integer.parseInt(varName.substring(3));
return textHierarchy[index];
}
throw new IllegalArgumentException("Unexpected variable: " + varName);
});
List<Choice> choices;
try { try {
NodeList results = (NodeList) xpath.evaluate(xpathExpression, vocabulary, XPathConstants.NODESET); NodeList results = (NodeList) xpath.evaluate(xpathExpression, vocabulary, XPathConstants.NODESET);
choices = getChoicesFromNodeList(results, 0, 1); choices = getChoicesFromNodeList(results, 0, 1);
@@ -258,7 +277,7 @@ public class DSpaceControlledVocabulary extends SelfNamedPlugin implements Hiera
@Override @Override
public Choices getChoicesByParent(String authorityName, String parentId, int start, int limit, String locale) { public Choices getChoicesByParent(String authorityName, String parentId, int start, int limit, String locale) {
init(locale); init(locale);
String xpathExpression = String.format(idTemplate, parentId); String xpathExpression = String.format(idTemplateQuoted, parentId);
return getChoicesByXpath(xpathExpression, start, limit); return getChoicesByXpath(xpathExpression, start, limit);
} }
@@ -282,15 +301,12 @@ public class DSpaceControlledVocabulary extends SelfNamedPlugin implements Hiera
} }
private boolean isRootElement(Node node) { private boolean isRootElement(Node node) {
if (node != null && node.getOwnerDocument().getDocumentElement().equals(node)) { return node != null && node.getOwnerDocument().getDocumentElement().equals(node);
return true;
}
return false;
} }
private Node getNode(String key, String locale) throws XPathExpressionException { private Node getNode(String key, String locale) throws XPathExpressionException {
init(locale); init(locale);
String xpathExpression = String.format(idTemplate, key); String xpathExpression = String.format(idTemplateQuoted, key);
Node node = getNodeFromXPath(xpathExpression); Node node = getNodeFromXPath(xpathExpression);
return node; return node;
} }
@@ -302,7 +318,7 @@ public class DSpaceControlledVocabulary extends SelfNamedPlugin implements Hiera
} }
private List<Choice> getChoicesFromNodeList(NodeList results, int start, int limit) { private List<Choice> getChoicesFromNodeList(NodeList results, int start, int limit) {
List<Choice> choices = new ArrayList<Choice>(); List<Choice> choices = new ArrayList<>();
for (int i = 0; i < results.getLength(); i++) { for (int i = 0; i < results.getLength(); i++) {
if (i < start) { if (i < start) {
continue; continue;
@@ -321,17 +337,17 @@ public class DSpaceControlledVocabulary extends SelfNamedPlugin implements Hiera
private Map<String, String> addOtherInformation(String parentCurr, String noteCurr, private Map<String, String> addOtherInformation(String parentCurr, String noteCurr,
List<String> childrenCurr, String authorityCurr) { List<String> childrenCurr, String authorityCurr) {
Map<String, String> extras = new HashMap<String, String>(); Map<String, String> extras = new HashMap<>();
if (StringUtils.isNotBlank(parentCurr)) { if (StringUtils.isNotBlank(parentCurr)) {
extras.put("parent", parentCurr); extras.put("parent", parentCurr);
} }
if (StringUtils.isNotBlank(noteCurr)) { if (StringUtils.isNotBlank(noteCurr)) {
extras.put("note", noteCurr); extras.put("note", noteCurr);
} }
if (childrenCurr.size() > 0) { if (childrenCurr.isEmpty()) {
extras.put("hasChildren", "true");
} else {
extras.put("hasChildren", "false"); extras.put("hasChildren", "false");
} else {
extras.put("hasChildren", "true");
} }
extras.put("id", authorityCurr); extras.put("id", authorityCurr);
return extras; return extras;
@@ -386,7 +402,7 @@ public class DSpaceControlledVocabulary extends SelfNamedPlugin implements Hiera
} }
private List<String> getChildren(Node node) { private List<String> getChildren(Node node) {
List<String> children = new ArrayList<String>(); List<String> children = new ArrayList<>();
NodeList childNodes = node.getChildNodes(); NodeList childNodes = node.getChildNodes();
for (int ci = 0; ci < childNodes.getLength(); ci++) { for (int ci = 0; ci < childNodes.getLength(); ci++) {
Node firstChild = childNodes.item(ci); Node firstChild = childNodes.item(ci);
@@ -409,7 +425,7 @@ public class DSpaceControlledVocabulary extends SelfNamedPlugin implements Hiera
private boolean isSelectable(Node node) { private boolean isSelectable(Node node) {
Node selectableAttr = node.getAttributes().getNamedItem("selectable"); Node selectableAttr = node.getAttributes().getNamedItem("selectable");
if (null != selectableAttr) { if (null != selectableAttr) {
return Boolean.valueOf(selectableAttr.getNodeValue()); return Boolean.parseBoolean(selectableAttr.getNodeValue());
} else { // Default is true } else { // Default is true
return true; return true;
} }
@@ -436,7 +452,7 @@ public class DSpaceControlledVocabulary extends SelfNamedPlugin implements Hiera
} }
private Choices getChoicesByXpath(String xpathExpression, int start, int limit) { private Choices getChoicesByXpath(String xpathExpression, int start, int limit) {
List<Choice> choices = new ArrayList<Choice>(); List<Choice> choices = new ArrayList<>();
XPath xpath = XPathFactory.newInstance().newXPath(); XPath xpath = XPathFactory.newInstance().newXPath();
try { try {
Node parentNode = (Node) xpath.evaluate(xpathExpression, vocabulary, XPathConstants.NODE); Node parentNode = (Node) xpath.evaluate(xpathExpression, vocabulary, XPathConstants.NODE);

20
dspace-api/src/test/java/org/dspace/content/authority/DSpaceControlledVocabularyTest.java
View File

@@ -8,6 +8,7 @@
package org.dspace.content.authority; package org.dspace.content.authority;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotNull;
import java.io.IOException; import java.io.IOException;
@@ -86,6 +87,7 @@ public class DSpaceControlledVocabularyTest extends AbstractDSpaceTest {
CoreServiceFactory.getInstance().getPluginService().getNamedPlugin(Class.forName(PLUGIN_INTERFACE), "farm"); CoreServiceFactory.getInstance().getPluginService().getNamedPlugin(Class.forName(PLUGIN_INTERFACE), "farm");
assertNotNull(instance); assertNotNull(instance);
Choices result = instance.getMatches(text, start, limit, locale); Choices result = instance.getMatches(text, start, limit, locale);
assertNotEquals("At least one match expected", 0, result.values.length);
assertEquals("north 40", result.values[0].value); assertEquals("north 40", result.values[0].value);
} }
@@ -110,6 +112,7 @@ public class DSpaceControlledVocabularyTest extends AbstractDSpaceTest {
"countries"); "countries");
assertNotNull(instance); assertNotNull(instance);
Choices result = instance.getMatches(labelPart, start, limit, null); Choices result = instance.getMatches(labelPart, start, limit, null);
assertNotEquals("At least one match expected", 0, result.values.length);
assertEquals(idValue, result.values[0].value); assertEquals(idValue, result.values[0].value);
assertEquals("Algeria", result.values[0].label); assertEquals("Algeria", result.values[0].label);
} }
@@ -132,14 +135,16 @@ public class DSpaceControlledVocabularyTest extends AbstractDSpaceTest {
"countries"); "countries");
assertNotNull(instance); assertNotNull(instance);
Choices result = instance.getBestMatch(idValue, null); Choices result = instance.getBestMatch(idValue, null);
assertNotEquals("At least one match expected", 0, result.values.length);
assertEquals(idValue, result.values[0].value); assertEquals(idValue, result.values[0].value);
assertEquals("Algeria", result.values[0].label); assertEquals("Algeria", result.values[0].label);
} }
/** /**
* Test of getMatches method of class * Test of getMatches method of class DSpaceControlledVocabulary
* DSpaceControlledVocabulary using a localized controlled vocabulary with valid locale parameter (localized * using a localized controlled vocabulary with valid locale parameter
* label returned) * (localized label returned).
* @throws java.lang.ClassNotFoundException if class under test cannot be found.
*/ */
@Test @Test
public void testGetMatchesGermanLocale() throws ClassNotFoundException { public void testGetMatchesGermanLocale() throws ClassNotFoundException {
@@ -157,14 +162,16 @@ public class DSpaceControlledVocabularyTest extends AbstractDSpaceTest {
"countries"); "countries");
assertNotNull(instance); assertNotNull(instance);
Choices result = instance.getMatches(labelPart, start, limit, "de"); Choices result = instance.getMatches(labelPart, start, limit, "de");
assertNotEquals("At least one match expected", 0, result.values.length);
assertEquals(idValue, result.values[0].value); assertEquals(idValue, result.values[0].value);
assertEquals("Algerien", result.values[0].label); assertEquals("Algerien", result.values[0].label);
} }
/** /**
* Test of getBestMatch method of class * Test of getBestMatch method of class DSpaceControlledVocabulary
* DSpaceControlledVocabulary using a localized controlled vocabulary with valid locale parameter (localized * using a localized controlled vocabulary with valid locale parameter
* label returned) * (localized label returned).
* @throws java.lang.ClassNotFoundException if class under test cannot be found.
*/ */
@Test @Test
public void testGetBestMatchIdValueGermanLocale() throws ClassNotFoundException { public void testGetBestMatchIdValueGermanLocale() throws ClassNotFoundException {
@@ -179,6 +186,7 @@ public class DSpaceControlledVocabularyTest extends AbstractDSpaceTest {
"countries"); "countries");
assertNotNull(instance); assertNotNull(instance);
Choices result = instance.getBestMatch(idValue, "de"); Choices result = instance.getBestMatch(idValue, "de");
assertNotEquals("At least one match expected", 0, result.values.length);
assertEquals(idValue, result.values[0].value); assertEquals(idValue, result.values[0].value);
assertEquals("Algerien", result.values[0].label); assertEquals("Algerien", result.values[0].label);
} }