antville/helma
antville/helma
1
1
Fork 0
Code Issues 5 Pull requests 7 Projects Releases 5 Activity Actions

* Apply better protection against SQL injection following bug 577

Browse source 
<http://helma.org/bugs/show_bug.cgi?id=577> by introducing 
  DbMapping.checkNumber() to catch anything not looking like a number literal 
  from being used as such. I't quite likely this breaks for some SQL type and 
  app out there, but it's better to be careful here.
* Introduce new DbMapping.virtual/DbMapping.isVirtual() flag that easily lets us 
  know whether a Node/DbMapping is a virtual one (collection, mountpoint etc)
  Using this in Node.getNonVirtualParent() fixes bug 566
  <http://helma.org/bugs/show_bug.cgi?id=566>.
* Remove obsolete and unused Node.setParent(Node, String).
* Set parentHandle directly instead of calling setParent(Node) in Node.getParent().
This commit is contained in:
hns 2007-11-28 15:32:09 +00:00
parent 230469d544
commit 1e783a8c99
4 changed files with 69 additions and 94 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

62
src/helma/objectmodel/db/DbMapping.java
View file

@ -123,11 +123,22 @@ public final class DbMapping {
// Set of mappings that depend on us and should be forwarded last data change events
HashSet dependentMappings = new HashSet();
// does this DbMapping describe a virtual node (collection, mountpoint, groupnode)?
private boolean virtual = false;
/**
* Create an empty DbMapping
* Create an internal DbMapping used for "virtual" mappings aka collections, mountpoints etc.
*/
public DbMapping(Application app, String typename) {
this(app, typename, null);
public DbMapping(Application app, String parentTypeName) {
this(app, parentTypeName, null);
// DbMappings created with this constructor always define virtual nodes
virtual = true;
if (parentTypeName != null) {
parentMapping = app.getDbMapping(parentTypeName);
if (parentMapping == null) {
throw new IllegalArgumentException("Unknown parent mapping: " + parentTypeName);
}
}
}
/**
@ -764,12 +775,7 @@ public final class DbMapping {
// if mapping doesn' exist or isn't defined, create a new (anonymous internal) one
groupbyMapping = new DbMapping(app, subRelation.groupbyPrototype);
// If a mapping is defined, make the internal mapping inherit from
// the defined named prototype.
if (subRelation.groupbyPrototype != null) {
groupbyMapping.parentMapping = app.getDbMapping(subRelation.groupbyPrototype);
}
// set subnode and property relations
groupbyMapping.subRelation = subRelation.getGroupbySubnodeRelation();
if (propRelation != null) {
@ -1503,13 +1509,13 @@ public final class DbMapping {
for (int i = 0; i < values.length; i++) {
if (i > 0)
q.append(", ");
q.append("'").append(escape(values[i])).append("'");
q.append("'").append(escapeString(values[i])).append("'");
}
} else {
for (int i = 0; i < values.length; i++) {
if (i > 0)
q.append(", ");
q.append(values[i]);
q.append(checkNumber(values[i]));
}
}
q.append(")");
@ -1531,9 +1537,9 @@ public final class DbMapping {
q.append(column).append(" = ");
if (needsQuotes(column)) {
q.append("'").append(escape(val)).append("'");
q.append("'").append(escapeString(val)).append("'");
} else {
q.append(val);
q.append(checkNumber(val));
}
}
@ -1544,7 +1550,8 @@ public final class DbMapping {
* @param str the string to escape
* @return the escaped string
*/
static String escape(String str) {
static String escapeString(Object value) {
String str = value == null ? null : value.toString();
if (str == null) {
return null;
} else if (str.indexOf("'") < 0) {
@ -1564,4 +1571,31 @@ public final class DbMapping {
}
return sbuf.toString();
}
/**
* Utility method to check whether the argument is a number literal.
* @param str a string representing a number literal
* @return the argument, if it conforms to the number literal syntax
* @throws IllegalArgumentException if the argument does not represent a number
*/
static String checkNumber(Object value) throws IllegalArgumentException {
String str = value == null ? null : value.toString();
if (str == null) {
return null;
} else {
str = str.trim();
if (str.matches("(?:\\+|\\-)??\\d+(?:\\.\\d+)??")) {
return str;
}
}
throw new IllegalArgumentException("Illegal numeric literal: " + str);
}
/**
* Find if this DbMapping describes a virtual node (collection, mountpoint, groupnode)
* @return true if this instance describes a virtual node.
*/
public boolean isVirtual() {
return virtual;
}
}

73
src/helma/objectmodel/db/Node.java
View file

@ -691,64 +691,6 @@ public final class Node implements INode, Serializable {
parentHandle = parent;
}
/**
* This version of setParent additionally marks the node as anonymous or non-anonymous,
* depending on the string argument. This is the version called from the scripting framework,
* while the one argument version is called from within the objectmodel classes only.
*/
public void setParent(Node parent, String propertyName) {
// we only do that for relational nodes.
if (!isRelational()) {
return;
}
NodeHandle oldParentHandle = parentHandle;
parentHandle = (parent == null) ? null : parent.getHandle();
// mark parent as set, otherwise getParent will try to
// determine the parent again when called.
lastParentSet = System.currentTimeMillis();
if ((parentHandle == null) || parentHandle.equals(oldParentHandle)) {
// nothing changed, no need to find access property
return;
}
if ((parent != null) && (propertyName == null)) {
// see if we can find out the propertyName by ourselfes by looking at the
// parent's property relation
String newname = null;
DbMapping parentmap = parent.getDbMapping();
if (parentmap != null) {
// first try to retrieve name via generic property relation of parent
Relation prel = parentmap.getSubnodeRelation();
if ((prel != null) && (prel.otherType == dbmap) &&
(prel.accessName != null)) {
// reverse look up property used to access this via parent
Relation proprel = dbmap.columnNameToRelation(prel.accessName);
if ((proprel != null) && (proprel.propName != null)) {
newname = getString(proprel.propName);
}
}
}
// did we find a new name for this
if (newname == null) {
this.anonymous = true;
} else {
this.anonymous = false;
this.name = newname;
}
} else {
this.anonymous = false;
this.name = propertyName;
}
}
/**
* Get parent, retrieving it if necessary.
*/
@ -794,7 +736,7 @@ public final class Node implements INode, Serializable {
} else if (pn2.equals(this)) {
// a special case we want to support: virtualname is actually
// a reference to this node, not a collection containing this node.
setParent(pn);
parentHandle = pn.getHandle();
name = pinfo.virtualname;
anonymous = false;
return pn;
@ -813,7 +755,7 @@ public final class Node implements INode, Serializable {
}
if (pn != null) {
setParent(pn);
parentHandle = pn.getHandle();
lastParentSet = System.currentTimeMillis();
return pn;
@ -826,7 +768,7 @@ public final class Node implements INode, Serializable {
if (i == parentInfo.length-1) {
// if we came till here and we didn't find a parent.
// set parent to null.
setParent(null);
parentHandle = null;
lastParentSet = System.currentTimeMillis();
}
}
@ -836,11 +778,9 @@ public final class Node implements INode, Serializable {
}
}
// fall back to heuristic parent (the node that fetched this one from db)
if (parentHandle == null) {
return null;
}
return parentHandle.getNode(nmgr);
}
@ -890,7 +830,7 @@ public final class Node implements INode, Serializable {
node.checkWriteLock();
}
// if subnodes are defined via realation, make sure its constraints are enforced.
// if subnodes are defined via relation, make sure its constraints are enforced.
if ((dbmap != null) && (dbmap.getSubnodeRelation() != null)) {
dbmap.getSubnodeRelation().setConstraints(this, node);
}
@ -1203,6 +1143,7 @@ public final class Node implements INode, Serializable {
// but it currently isn't supported by NodeManager.
// if (dbmap != null && dbmap.getSubnodeRelation () != null)
// retval = nmgr.getNode (this, subid, dbmap.getSubnodeRelation ());
if ((retval != null) && (retval.parentHandle == null) &&
!nmgr.isRootNode(retval)) {
retval.setParent(this);
@ -2531,7 +2472,7 @@ public final class Node implements INode, Serializable {
*/
public String toString() {
try {
// We need to reach deap into helma.framework.core to invoke onInit(),
// We need to reach deap into helma.framework.core to invoke toString(),
// but the functionality is really worth it.
RequestEvaluator reval = getApp().getCurrentRequestEvaluator();
if (reval != null) {
@ -2675,7 +2616,7 @@ public final class Node implements INode, Serializable {
if (node.getState() == Node.TRANSIENT) {
DbMapping map = node.getDbMapping();
if (map == null || map.getTypeName() != null)
if (map == null || !map.isVirtual())
return node;
} else if (node.getState() != Node.VIRTUAL) {
return node;

20
src/helma/objectmodel/db/Relation.java
View file

@ -734,18 +734,18 @@ public final class Relation {
return null;
}
// if the collection node is prototyped, return the app's DbMapping
// for that prototype
if (prototype != null) {
return otherType;
}
// create a synthetic DbMapping that describes how to fetch the
// collection's child objects.
if (virtualMapping == null) {
virtualMapping = new DbMapping(ownType.app, null);
virtualMapping.subRelation = getVirtualSubnodeRelation();
virtualMapping.propRelation = getVirtualPropertyRelation();
// if the collection node is prototyped (a mountpoint), create
// a virtual sub-mapping from the app's DbMapping for that prototype
if (prototype != null) {
virtualMapping = new DbMapping(ownType.app, prototype);
} else {
virtualMapping = new DbMapping(ownType.app, null);
virtualMapping.subRelation = getVirtualSubnodeRelation();
virtualMapping.propRelation = getVirtualPropertyRelation();
}
}
return virtualMapping;
@ -924,7 +924,7 @@ public final class Relation {
}
// end column version
if (value != null) {
q.append(DbMapping.escape(value.toString()));
q.append(DbMapping.escapeString(value.toString()));
} else {
q.append("NULL");
}

8
src/helma/objectmodel/db/UpdateableSubnodeList.java
View file

@ -149,9 +149,9 @@ public class UpdateableSubnodeList extends OrderedSubnodeList {
DbColumn dbc = rel.otherType.getColumn(updateCriteria[idx]);
if (rel.otherType.getIDField().equalsIgnoreCase(updateCriteria[idx])) {
if (rel.otherType.needsQuotes(updateCriteria[idx])) {
sb.append ("'").append (values[idx]).append("'");
sb.append("'").append(DbMapping.escapeString(values[idx])).append("'");
} else {
sb.append (values[idx]);
sb.append(DbMapping.checkNumber(values[idx]));
}
return;
}
@ -192,9 +192,9 @@ public class UpdateableSubnodeList extends OrderedSubnodeList {
case Types.CLOB:
default:
if (rel.otherType.needsQuotes(updateCriteria[idx])) {
sb.append ("'").append (strgVal).append ("'");
sb.append("'").append(DbMapping.escapeString(strgVal)).append("'");
} else {
sb.append (strgVal);
sb.append(DbMapping.checkNumber(strgVal));
}
}
}