OWASP - User contributions [en]

Hibernate-Guidelines

2008-11-05T18:46:25Z

Aldosolis: /* Passwords in the hibernate.cfg.xml in plain text */

== Status ==
'''In progress'''

== Important ==
=== SQL Injection ===

It is commonly thought that ORM layers, like Hibernate are immune to SQL injection. This is not the case as Hibernate includes a subset of SQL called HQL, and allows "native" SQL queries. Often the ORM layer only minimally manipulates the inbound query before handing it off to the database for processing.

=== Transaction Scope ===

All database communication should occur within the scope of a Transaction.

=== Persisting Tainted Data ===

If an application sets tainted properties in an object, then makes that object persistent, it is highly likely that data will be accessed at some point and displayed. If database values are never displayed to the user then obviously this is nullified. Better yet if we could figure out which values from the db are tainted but I have a feeling it's not a feasible scan.

=== Rollback ===

When an exception occurs, roll back the Transaction and close the Session. If you don't, Hibernate can't guarantee that in-memory state accurately represents persistent state.
If your Session is bound to the application, you have to stop the application. Rolling back the database transaction doesn't put your business objects back into the state they were at the start of the transaction, so the database state and the business objects do get out of sync. Objects that are manipulated after a rollback as if they were synchronized.

----

== Simple ==

=== Don't use load() to determine existence ===
Do not use Session.load() to determine if an instance with the given identifier exists on the database; use Session.get() or a query instead. This is a poss

=== Constructing Query Strings ===
Hibernate has a set of functions that are used internally to construct their query strings, but there is no limit to using them in application code - what if they were used with tainted data? This is pretty self explanatory.

=== Place each class mapping in its own file ===
Don't use a single monolithic mapping document. Map com.eg.Foo in the file com/eg/Foo.hbm.xml. This makes particularly good sense in a team environment. Nothing more than a app dev good practice.

=== Use bind variables ===
As in JDBC, always replace non-constant values by "?". Never use string manipulation to bind a nonconstant value in a query! Even better, consider using named parameters in queries.

=== Load mappings as resources ===
Deploy the mappings along with the classes they map.

=== Passwords in the hibernate.cfg.xml in plain text ===
Sensitive information such as passwords should be encrypted to prevent attacks like connecting directly to the database, you can extend Hibernate functionality to implement password encryption.

----

== Detailed ==

=== Transaction Timeout ===
*Transaction timeouts ensure that no misbehaving transaction can indefinitely tie up resources while returning no response to the user.
*Outside a managed (JTA) environment, Hibernate cannot fully provide this functionality. However, Hibernate can at least control data access operations, ensuring that database level deadlocks and queries with huge result sets are limited by a defined timeout.
*In a managed environment, Hibernate can delegate transaction timeout to JTA.

<pre>
Session sess = factory.openSession();
try {
//set transaction timeout to 3 seconds
sess.getTransaction().setTimeout(3);
sess.getTransaction().begin();
// do some work
...
sess.getTransaction().commit()
}
catch (RuntimeException e) {
sess.getTransaction().rollback();
throw e; // or display error message
}
finally {
sess.close();
}
</pre>

===Identify natural keys ===
*Even though we recommend the use of surrogate keys as primary keys, you should still try to identify natural keys for all entities.
*A natural key is a property or combination of properties that is unique and non-null. If it is also immutable, even better.
*Map the properties of the natural key inside the <natural-id> element. Hibernate will generate the necessary unique key and nullability constraints, and your mapping will be more selfdocumenting.
*We strongly recommend that you implement equals() and hashCode() to compare the natural key properties of the entity.
*This mapping is not intended for use with entities with natural primary keys..

Example in hbm.xml:
<pre>
<natural-id mutable="true|false"/>
<property ... />
<many-to-one ... />
......
</natural-id>
</pre>

=== Parameter Binding ===

Hibernate parameter binding works like prepared statements. An edge case it is, but say you had an already tainted string, and then used Query setters to bind more parameters to the "?" placeholders.

Example of proper use:
<pre>
Query q = sess.createQuery("from DomesticCat cat where cat.name = ?");
q.setString(0, "Izi");
Iterator cats = q.iterate();
</pre>

Improper use:
<pre>
Insert insert = new Insert(dialect);
insert.setComment(taint);
insert.addSelectColumn(columnName, alias);
String sql = insert.toQueryString();
....
Query query = sess.createQuery(sql); /*B00*///creating a Query with an already tainted string
query.setString(position, val); //this doesn't cleanse the damage that's been done.
....
transaction.commit(); //sink the ship
</pre>

=== Declare identifier properties on persistent classes ===

Hibernate makes identifier properties optional. There are all sorts of reasons why you should use them. Mapped classes must declare the primary key column of the database table. Most classes will also have a Java-Beans-style property holding the unique identifier of an instance. This is the easiest and most recommended route.
{{{
<class name="Summary">
<subselect>
select item.name, max(bid.amount), count(*)
from item
join bid on bid.item_id = item.id
group by item.name
</subselect>
<synchronize table="item"/>
<synchronize table="bid"/>
<id
name="propertyName" (1)
type="typename" (2)
column="column_name" (3)
unsaved-value="null|any|none|undefined|id_value" (4)
access="field|property|ClassName"> (5)
node="element-name|@attribute-name|element/@attribute|."
<generator class="generatorClass"/>
</id>
</class>
}}}

=== Don't use session.find() ===
This pretty much looks like a duplicate of sql injection but using the deprecated method find() instead. This rule was pulled straight from the best practice list from the reference manual.

If using Hibernate, do not use the depreciated session.find() method without using one of the query binding overloads. Using session.find() with direct user input allows the user input to be passed directly to the underlying SQL engine and will result in SQL injections on all supported RDBMS.

<pre>

Payment payment = (Payment) session.
find("from com.example.Payment as payment where payment.id = " + paymentIds.get(i));
</pre>

The above Hibernate HQL will allow SQL injection from paymentIds, which are obtained from the user. A safer way to express this is:

<pre>

int pId = paymentIds.get(i);

TsPayment payment = (TsPayment) session.
find("from com.example.Payment as payment where payment.id = ?", pId, StringType);

</pre>

=== Cache Management and !OutOfMemoryException ===

*The Session caches every object that is in persistent state (watched and checked for dirty state by Hibernate).
*Whenever you pass an object to save(), update() or saveOrUpdate() and whenever you retrieve an object using load(), get(), list(), iterate() or scroll(), that object is added to the internal cache of the Session.
*When flush() or commit() is subsequently called, the state of that object will be synchronized with the database.
*Otherwise it grows endlessly until you get an OutOfMemoryException, if you keep it open for a long time or simply load too much data.
*One solution for this is to call clear() and evict() to manage the Session cache, but you most likely should consider a Stored Procedure if you need mass data operations.
*Some solutions are shown in Chapter 13, Batch processing. Keeping a Session open for the duration of a user session also means a high probability of stale data.

In this example, it would fall over with an OutOfMemoryException somewhere around the 50 000th row. That's because Hibernate caches all the newly inserted Customer instances in the session-level cache.
<pre>
Session session = sessionFactory.openSession();
Transaction tx = session.beginTransaction();
for ( int i=0; i<100000; i++ ) {
Customer customer = new Customer(.....);
session.save(customer);
}
tx.commit();
session.close();
</pre>
When making new objects persistent, you must flush() and then clear() the session regularly, to control the

<pre>
size of the first-level cache.
Session session = sessionFactory.openSession();
Transaction tx = session.beginTransaction();
for ( int i=0; i<100000; i++ ) {
Customer customer = new Customer(.....);
session.save(customer);
if ( i % 20 == 0 ) { //20, same as the JDBC batch size
//flush a batch of inserts and release memory:
session.flush();
session.clear();
}
}
tx.commit();
session.close();
</pre>

=== About StatelessSession ===
*A StatelessSession has no persistence context associated with it and does not provide many of the higher-level life cycle semantics. In particular, a stateless session does not implement a first-level cache nor interact with any second-level or query cache.
*This may be used as a solution for the cache out of memory problem.
*Operations performed using a stateless session do not ever cascade to associated instances. Collections are ignored by a stateless session.
*The insert(), update() and delete() operations defined by the StatelessSession interface are considered
to be direct database row-level operations, which result in immediate execution of a SQL INSERT, UPDATE or
DELETE respectively.

Example of proper use:
<pre>
StatelessSession session = sessionFactory.openStatelessSession();
Transaction tx = session.beginTransaction();

ScrollableResults customers = session.getNamedQuery("GetCustomers")
.scroll(ScrollMode.FORWARD_ONLY);
while ( customers.next() ) {
Customer customer = (Customer) customers.get(0);
customer.updateStuff(...);
session.update(customer);
}
tx.commit();
session.close();
</pre>

=== Aboot POJOs ===
* You can tell what objects are the persistent ones by looking at the xxx.hbm.xml files.
* Persistent objects are usually POJO's with a default no-arg constructor so that Hibernate can instantiate them using Constructor.newInstance().
* It is recommended that these POJO's have a property that is explicitly mapped as a primary key.

<pre>
<hibernate-mapping>
<class entity-name="Customer">
<id name="id"
type="long"
column="ID">
<generator class="sequence"/>
</id>
......
</pre>

* Accessors and mutators should be declared for persistent fields - it is optional but is good practice because Hibernate persists JavaBeans style properties.

=== Session and Concurrency issues ===

*A Session is not thread-safe.
*Things which are supposed to work concurrently, like HTTP requests, session beans, or Swing workers, will cause race conditions if a Session instance would be shared.
*If you keep your Hibernate Session in your HttpSession, you should consider synchronizing access to your Http session.
*Otherwise, a user that clicks reload fast enough may use the same Session in two concurrently running threads.

=== The equals() and hashCode() problem: ===

*Most Java objects provide a built-in equals() and hashCode() based on the object's identity; so each new() object will be different from all others.
*If all your objects are in memory, this is a fine model, but Hibernate's whole job, of course, is to move your objects out of memory.
* Hibernate uses the Hibernate session to manage this uniqueness. When you create an object with new(), and then save it into a session, Hibernate now knows that whenever you query for an object and find that particular object, Hibernate should return you that instance of the object.
* However, once you close the Hibernate session, all bets are off. If you keep holding onto an object that you either created or loaded in a Hibernate session that you have now closed, Hibernate has no way to know about those objects.
* So if you open another session and query for "the same" object, Hibernate will return you a new instance. Hence, if you keep collections of objects around between sessions, you will start to experience odd behavior (duplicate objects in collections, mainly).
* '''Long story short equals() and hashCode() should be implemented to compare object properties and not just compare on the basis of identifier (the property mapped as the primary key see the above example). The problem is discussed in detail [http://www.hibernate.org/109.html here].'''

<pre>
public class Cat {
...
public boolean equals(Object other) {
if (this == other) return true;
if ( !(other instanceof Cat) ) return false;
final Cat cat = (Cat) other;
if ( !cat.getLitterId().equals( getLitterId() ) ) return false;
if ( !cat.getMother().equals( getMother() ) ) return false;
return true;
}
public int hashCode() {
int result;
result = getMother().hashCode();
result = 29 * result + getLitterId();
return result;
}
}
</pre>

=== Using Detached Objects ===
*In a three tiered architecture, consider using detached objects.
*When using a servlet / session bean architecture, you could pass persistent objects loaded in the session
bean to and from the servlet / JSP layer.
*Use a new session to service each request.
*Use Session.merge() or Session.saveOrUpdate() to synchronize objects with the database.
Usually update() or saveOrUpdate() are used in the following scenario:
*the application loads an object in the first session
*the object is passed up to the UI tier
*some modifications are made to the object
*the object is passed back down to the business logic tier
*the application persists these modifications by calling update() in a second session

<pre>
// in the first session
Cat cat = (Cat) firstSession.load(Cat.class, catID);
// in a higher tier of the application
Cat mate = new Cat();
cat.setMate(mate);
// later, in a new session
secondSession.saveOrUpdate(cat); // update existing state (cat has a non-null id)
secondSession.saveOrUpdate(mate); // save the new instance (mate has a null id)
</pre>

=== Consider externalising query strings ===

This is a good practice if your queries call non-ANSI-standard SQL functions. Externalising the query
strings to mapping files will make the application more portable.
It also decreases the likelihood of injection because the Query objects let you set individual parameters but not the entire query string - you can only do that when you pass the string into the Query implementation constructor or call Session.createQuery.
==== Edge cases are still possibilities! ====
*Basically the application would have to be messy enough to be binding parameters here, and concatenating variables there to the same query string.
*You can extract the query string using Query.getQueryString(), and although you can't manipulate the string and 'add' it back to the Query object, you could theoretically taint it and use it in another instance of Query. Dumb? yes. Possible? Yes.

==== So how to externalize? ====
You may define named queries in the mapping document.

<pre>
<query name="ByNameAndMaximumWeight"><![CDATA[
from eg.DomesticCat as cat
where cat.name = ?
and cat.weight > ?
] ]></query>
</pre>

Parameter binding and executing is done programatically:

<pre>
Query q = sess.getNamedQuery("ByNameAndMaximumWeight");
q.setString(0, name);
q.setInt(1, minWeight);
List cats = q.list();
</pre>

*Note that the actual program code is independent of the query language that is used, you may also define native
SQL queries in metadata, or migrate existing queries to Hibernate by placing them in mapping files.
*Also note that a query declaration inside a <hibernate-mapping> element requires a global unique name for the
query, while a query declaration inside a <class> element is made unique automatically by prepending the
fully qualified name of the class, for example eg.Cat.ByNameAndMaximumWeight.

[[Category:Hibernate]]
[[Category:OWASP Java Project]]
[[Category:Java]]

Hibernate-Guidelines

2008-11-05T18:45:49Z

Aldosolis: /* Simple */

== Status ==
'''In progress'''

== Important ==
=== SQL Injection ===

It is commonly thought that ORM layers, like Hibernate are immune to SQL injection. This is not the case as Hibernate includes a subset of SQL called HQL, and allows "native" SQL queries. Often the ORM layer only minimally manipulates the inbound query before handing it off to the database for processing.

=== Transaction Scope ===

All database communication should occur within the scope of a Transaction.

=== Persisting Tainted Data ===

If an application sets tainted properties in an object, then makes that object persistent, it is highly likely that data will be accessed at some point and displayed. If database values are never displayed to the user then obviously this is nullified. Better yet if we could figure out which values from the db are tainted but I have a feeling it's not a feasible scan.

=== Rollback ===

When an exception occurs, roll back the Transaction and close the Session. If you don't, Hibernate can't guarantee that in-memory state accurately represents persistent state.
If your Session is bound to the application, you have to stop the application. Rolling back the database transaction doesn't put your business objects back into the state they were at the start of the transaction, so the database state and the business objects do get out of sync. Objects that are manipulated after a rollback as if they were synchronized.

----

== Simple ==

=== Don't use load() to determine existence ===
Do not use Session.load() to determine if an instance with the given identifier exists on the database; use Session.get() or a query instead. This is a poss

=== Constructing Query Strings ===
Hibernate has a set of functions that are used internally to construct their query strings, but there is no limit to using them in application code - what if they were used with tainted data? This is pretty self explanatory.

=== Place each class mapping in its own file ===
Don't use a single monolithic mapping document. Map com.eg.Foo in the file com/eg/Foo.hbm.xml. This makes particularly good sense in a team environment. Nothing more than a app dev good practice.

=== Use bind variables ===
As in JDBC, always replace non-constant values by "?". Never use string manipulation to bind a nonconstant value in a query! Even better, consider using named parameters in queries.

=== Load mappings as resources ===
Deploy the mappings along with the classes they map.

=== Passwords in the hibernate.cfg.xml in plain text ===
Sensitive information such passwords should be encrypted to prevent attacks connecting directly to the database, you can extend Hibernate functionality to implement password encryption.

----

== Detailed ==

=== Transaction Timeout ===
*Transaction timeouts ensure that no misbehaving transaction can indefinitely tie up resources while returning no response to the user.
*Outside a managed (JTA) environment, Hibernate cannot fully provide this functionality. However, Hibernate can at least control data access operations, ensuring that database level deadlocks and queries with huge result sets are limited by a defined timeout.
*In a managed environment, Hibernate can delegate transaction timeout to JTA.

<pre>
Session sess = factory.openSession();
try {
//set transaction timeout to 3 seconds
sess.getTransaction().setTimeout(3);
sess.getTransaction().begin();
// do some work
...
sess.getTransaction().commit()
}
catch (RuntimeException e) {
sess.getTransaction().rollback();
throw e; // or display error message
}
finally {
sess.close();
}
</pre>

===Identify natural keys ===
*Even though we recommend the use of surrogate keys as primary keys, you should still try to identify natural keys for all entities.
*A natural key is a property or combination of properties that is unique and non-null. If it is also immutable, even better.
*Map the properties of the natural key inside the <natural-id> element. Hibernate will generate the necessary unique key and nullability constraints, and your mapping will be more selfdocumenting.
*We strongly recommend that you implement equals() and hashCode() to compare the natural key properties of the entity.
*This mapping is not intended for use with entities with natural primary keys..

Example in hbm.xml:
<pre>
<natural-id mutable="true|false"/>
<property ... />
<many-to-one ... />
......
</natural-id>
</pre>

=== Parameter Binding ===

Hibernate parameter binding works like prepared statements. An edge case it is, but say you had an already tainted string, and then used Query setters to bind more parameters to the "?" placeholders.

Example of proper use:
<pre>
Query q = sess.createQuery("from DomesticCat cat where cat.name = ?");
q.setString(0, "Izi");
Iterator cats = q.iterate();
</pre>

Improper use:
<pre>
Insert insert = new Insert(dialect);
insert.setComment(taint);
insert.addSelectColumn(columnName, alias);
String sql = insert.toQueryString();
....
Query query = sess.createQuery(sql); /*B00*///creating a Query with an already tainted string
query.setString(position, val); //this doesn't cleanse the damage that's been done.
....
transaction.commit(); //sink the ship
</pre>

=== Declare identifier properties on persistent classes ===

Hibernate makes identifier properties optional. There are all sorts of reasons why you should use them. Mapped classes must declare the primary key column of the database table. Most classes will also have a Java-Beans-style property holding the unique identifier of an instance. This is the easiest and most recommended route.
{{{
<class name="Summary">
<subselect>
select item.name, max(bid.amount), count(*)
from item
join bid on bid.item_id = item.id
group by item.name
</subselect>
<synchronize table="item"/>
<synchronize table="bid"/>
<id
name="propertyName" (1)
type="typename" (2)
column="column_name" (3)
unsaved-value="null|any|none|undefined|id_value" (4)
access="field|property|ClassName"> (5)
node="element-name|@attribute-name|element/@attribute|."
<generator class="generatorClass"/>
</id>
</class>
}}}

=== Don't use session.find() ===
This pretty much looks like a duplicate of sql injection but using the deprecated method find() instead. This rule was pulled straight from the best practice list from the reference manual.

If using Hibernate, do not use the depreciated session.find() method without using one of the query binding overloads. Using session.find() with direct user input allows the user input to be passed directly to the underlying SQL engine and will result in SQL injections on all supported RDBMS.

<pre>

Payment payment = (Payment) session.
find("from com.example.Payment as payment where payment.id = " + paymentIds.get(i));
</pre>

The above Hibernate HQL will allow SQL injection from paymentIds, which are obtained from the user. A safer way to express this is:

<pre>

int pId = paymentIds.get(i);

TsPayment payment = (TsPayment) session.
find("from com.example.Payment as payment where payment.id = ?", pId, StringType);

</pre>

=== Cache Management and !OutOfMemoryException ===

*The Session caches every object that is in persistent state (watched and checked for dirty state by Hibernate).
*Whenever you pass an object to save(), update() or saveOrUpdate() and whenever you retrieve an object using load(), get(), list(), iterate() or scroll(), that object is added to the internal cache of the Session.
*When flush() or commit() is subsequently called, the state of that object will be synchronized with the database.
*Otherwise it grows endlessly until you get an OutOfMemoryException, if you keep it open for a long time or simply load too much data.
*One solution for this is to call clear() and evict() to manage the Session cache, but you most likely should consider a Stored Procedure if you need mass data operations.
*Some solutions are shown in Chapter 13, Batch processing. Keeping a Session open for the duration of a user session also means a high probability of stale data.

In this example, it would fall over with an OutOfMemoryException somewhere around the 50 000th row. That's because Hibernate caches all the newly inserted Customer instances in the session-level cache.
<pre>
Session session = sessionFactory.openSession();
Transaction tx = session.beginTransaction();
for ( int i=0; i<100000; i++ ) {
Customer customer = new Customer(.....);
session.save(customer);
}
tx.commit();
session.close();
</pre>
When making new objects persistent, you must flush() and then clear() the session regularly, to control the

<pre>
size of the first-level cache.
Session session = sessionFactory.openSession();
Transaction tx = session.beginTransaction();
for ( int i=0; i<100000; i++ ) {
Customer customer = new Customer(.....);
session.save(customer);
if ( i % 20 == 0 ) { //20, same as the JDBC batch size
//flush a batch of inserts and release memory:
session.flush();
session.clear();
}
}
tx.commit();
session.close();
</pre>

=== About StatelessSession ===
*A StatelessSession has no persistence context associated with it and does not provide many of the higher-level life cycle semantics. In particular, a stateless session does not implement a first-level cache nor interact with any second-level or query cache.
*This may be used as a solution for the cache out of memory problem.
*Operations performed using a stateless session do not ever cascade to associated instances. Collections are ignored by a stateless session.
*The insert(), update() and delete() operations defined by the StatelessSession interface are considered
to be direct database row-level operations, which result in immediate execution of a SQL INSERT, UPDATE or
DELETE respectively.

Example of proper use:
<pre>
StatelessSession session = sessionFactory.openStatelessSession();
Transaction tx = session.beginTransaction();

ScrollableResults customers = session.getNamedQuery("GetCustomers")
.scroll(ScrollMode.FORWARD_ONLY);
while ( customers.next() ) {
Customer customer = (Customer) customers.get(0);
customer.updateStuff(...);
session.update(customer);
}
tx.commit();
session.close();
</pre>

=== Aboot POJOs ===
* You can tell what objects are the persistent ones by looking at the xxx.hbm.xml files.
* Persistent objects are usually POJO's with a default no-arg constructor so that Hibernate can instantiate them using Constructor.newInstance().
* It is recommended that these POJO's have a property that is explicitly mapped as a primary key.

<pre>
<hibernate-mapping>
<class entity-name="Customer">
<id name="id"
type="long"
column="ID">
<generator class="sequence"/>
</id>
......
</pre>

* Accessors and mutators should be declared for persistent fields - it is optional but is good practice because Hibernate persists JavaBeans style properties.

=== Session and Concurrency issues ===

*A Session is not thread-safe.
*Things which are supposed to work concurrently, like HTTP requests, session beans, or Swing workers, will cause race conditions if a Session instance would be shared.
*If you keep your Hibernate Session in your HttpSession, you should consider synchronizing access to your Http session.
*Otherwise, a user that clicks reload fast enough may use the same Session in two concurrently running threads.

=== The equals() and hashCode() problem: ===

*Most Java objects provide a built-in equals() and hashCode() based on the object's identity; so each new() object will be different from all others.
*If all your objects are in memory, this is a fine model, but Hibernate's whole job, of course, is to move your objects out of memory.
* Hibernate uses the Hibernate session to manage this uniqueness. When you create an object with new(), and then save it into a session, Hibernate now knows that whenever you query for an object and find that particular object, Hibernate should return you that instance of the object.
* However, once you close the Hibernate session, all bets are off. If you keep holding onto an object that you either created or loaded in a Hibernate session that you have now closed, Hibernate has no way to know about those objects.
* So if you open another session and query for "the same" object, Hibernate will return you a new instance. Hence, if you keep collections of objects around between sessions, you will start to experience odd behavior (duplicate objects in collections, mainly).
* '''Long story short equals() and hashCode() should be implemented to compare object properties and not just compare on the basis of identifier (the property mapped as the primary key see the above example). The problem is discussed in detail [http://www.hibernate.org/109.html here].'''

<pre>
public class Cat {
...
public boolean equals(Object other) {
if (this == other) return true;
if ( !(other instanceof Cat) ) return false;
final Cat cat = (Cat) other;
if ( !cat.getLitterId().equals( getLitterId() ) ) return false;
if ( !cat.getMother().equals( getMother() ) ) return false;
return true;
}
public int hashCode() {
int result;
result = getMother().hashCode();
result = 29 * result + getLitterId();
return result;
}
}
</pre>

=== Using Detached Objects ===
*In a three tiered architecture, consider using detached objects.
*When using a servlet / session bean architecture, you could pass persistent objects loaded in the session
bean to and from the servlet / JSP layer.
*Use a new session to service each request.
*Use Session.merge() or Session.saveOrUpdate() to synchronize objects with the database.
Usually update() or saveOrUpdate() are used in the following scenario:
*the application loads an object in the first session
*the object is passed up to the UI tier
*some modifications are made to the object
*the object is passed back down to the business logic tier
*the application persists these modifications by calling update() in a second session

<pre>
// in the first session
Cat cat = (Cat) firstSession.load(Cat.class, catID);
// in a higher tier of the application
Cat mate = new Cat();
cat.setMate(mate);
// later, in a new session
secondSession.saveOrUpdate(cat); // update existing state (cat has a non-null id)
secondSession.saveOrUpdate(mate); // save the new instance (mate has a null id)
</pre>

=== Consider externalising query strings ===

This is a good practice if your queries call non-ANSI-standard SQL functions. Externalising the query
strings to mapping files will make the application more portable.
It also decreases the likelihood of injection because the Query objects let you set individual parameters but not the entire query string - you can only do that when you pass the string into the Query implementation constructor or call Session.createQuery.
==== Edge cases are still possibilities! ====
*Basically the application would have to be messy enough to be binding parameters here, and concatenating variables there to the same query string.
*You can extract the query string using Query.getQueryString(), and although you can't manipulate the string and 'add' it back to the Query object, you could theoretically taint it and use it in another instance of Query. Dumb? yes. Possible? Yes.

==== So how to externalize? ====
You may define named queries in the mapping document.

<pre>
<query name="ByNameAndMaximumWeight"><![CDATA[
from eg.DomesticCat as cat
where cat.name = ?
and cat.weight > ?
] ]></query>
</pre>

Parameter binding and executing is done programatically:

<pre>
Query q = sess.getNamedQuery("ByNameAndMaximumWeight");
q.setString(0, name);
q.setInt(1, minWeight);
List cats = q.list();
</pre>

*Note that the actual program code is independent of the query language that is used, you may also define native
SQL queries in metadata, or migrate existing queries to Hibernate by placing them in mapping files.
*Also note that a query declaration inside a <hibernate-mapping> element requires a global unique name for the
query, while a query declaration inside a <class> element is made unique automatically by prepending the
fully qualified name of the class, for example eg.Cat.ByNameAndMaximumWeight.

[[Category:Hibernate]]
[[Category:OWASP Java Project]]
[[Category:Java]]

Reviewing Code for Error Handling

2007-10-03T14:01:02Z

Aldosolis: /* JAVA */

[[OWASP Code Review Guide Table of Contents]]__TOC__

==Error, Exception handling & Logging. ==

An important aspect of secure application development is to prevent information leakage. Error messages give an attacker great insight into the inner workings of an application.

The purpose of reviewing the Error Handling code is to assure the application fails safely under all possible error conditions, expected and unexpected. No sensitive information is presented to the user when an error occurs.

For example SQL injection is much tougher to successfully pull off without some healthy error messages. It lessens the attack footprint and our attacker would have to resort to use “blind SQL injection” which is more difficult and time consuming.

A well-planned error/exception handling strategy is important for three reasons:

# Good error handling does not give an attacker any information which is a means to an end, attacking the application
# A proper centralised error strategy is easier to maintain and reduces the chance of any uncaught errors “Bubbling up” to the front end of an application.
# Information leakage can lead to social engineering exploits.

Some development languages provide checked exceptions which mean that the compiler shall complain if an exception for a particular API call is not caught Java and C# are good examples of this.
Languages like C++ and C do not provide this safety net. Languages with checked exception handling still are prone to information leakage as not all types of error are checked for.

When an exception or error is thrown we also need to log this occurrence. Sometimes this is due to bad development, but it can be the result of an attack or some other service your application relies on failing.

All code paths that can cause an exception to be thrown should check for success in order for the exception not to be thrown.

To avoid a NullPointerException we should check is the object being accessed is not null.

==Generic error messages==

We should use a localized description string in every exception, a friendly error reason such as “System Error – Please try again later”. When the user sees an error message, it will be derived from this description string of the exception that was thrown, and never from the exception class which may contain a stack trace, line number where the error occurred, class name or method name.

Do not expose sensitive information in exception messages. Information such as paths on the local file system is considered privileged information; any system internal information should be hidden from the user. As mentioned before an attacker could use this information to gather private user information from the application or components that make up the app.

Don’t put people’s names or any internal contact information in error messages. Don’t put any “human” information, which would lead to a level of familiarity and a social engineering exploit.

==How to locate the potentially vulnerable code==

===JAVA===

In java we have the concept of an error object, the Exception object.
This lives in the java package java.lang and is derived from the Throwable object
Exceptions are thrown when an abnormal occurrence has occurred. Another object derived from Throwable is the Error object, which is thrown when something more serious occurs.

Information leakage can occur when developers use some exception methods, which ‘bubble’ to the user UI due to a poor error handling strategy.
The methods are as follows:
printStackTrace()
getStackTrace()

Also is important to know that the output of these methods is printed in System console, the same as System.out.println(e) where e is an Exception. Be sure to not redirect the outputStream to PrintWriter object of JSP, by convention called "out". Ex. printStackTrace(out);

Also another object to look at is the java.lang.system package:

setErr() and the System.err field.

===.NET===

In .NET a System.Exception object exists. Commonly used child objects such as ApplicationException and SystemException are used.
It is not recommended that you throw or catch a SystemException this is thrown by runtime.

When an error occurs, either the system or the currently executing application reports it by throwing an exception containing information about the error, similar to java. Once thrown, an exception is handled by the application or by the default exception handler.
This Exception object contains similar methods to the java implementation such as:

StackTrace
Source
Message
HelpLink

In .NET we need to look at the error handling strategy from the point of view of global error handling and the handling of unexpected errors. This can be done in many ways and this article is not an exhaustive list.
Firstly an Error Event is thrown when an unhandled exception is thrown. This is part of the TemplateControl class.

http://msdn.microsoft.com/library/default.asp?url=/library/en-us/cpref/html/frlrfSystemWebUITemplateControlClassErrorTopic.asp

==Error handling can be done in three ways in .NET==

*In the web.config file's customErrors section.
*In the global.asax file's Application_Error sub.
*On the aspx or associated codebehind page in the Page_Error sub

The order of error handling events in .NET is as follows:
# On the Page in the Page_Error sub.
# The global.asax Application_Error sub
# The web.config file

It is recommended to look in these areas to understand the error strategy of the application.

==Vulnerable Patterns for Error Handling==

===Page_Error===

Page_Error is page level handling which is run on the server side.
Below is an example but the error information is a little too informative and hence bad practice.

'''FIXME: code formatting'''

<nowiki>

<script language="C#" runat="server">

Sub Page_Error(Source As Object, E As EventArgs)

Dim message As String = "<font face=verdana color=red><h1>" & Request.Url.ToString()& "</h1>" &

"<pre><font color='red'>" & Server.GetLastError().ToString()& "</pre></font>"

Response.Write(message) // display message

End Sub

</script>

</nowiki>

The red text in the example above has a number of issues:
Firstly it redisplays the HTTP request to the user in the form of Request.Url.ToString() Assuming there has been no data validation prior to this point we are vulnerable to cross site scripting attacks!! Secondly the error message and stack trace is displayed to the user using Server.GetLastError().ToString() which divulges internal information regarding the application.

After the Page_Error is called, the Application_Error sub is called:

===Global.asax===

When an error occurs, the Application_Error sub is called. In this method we can
log the error and redirect to another page.

<%@ Import Namespace="System.Diagnostics" %>
<script language="C#" runat="server">
void Application_Error(Object sender, EventArgs e) {
String Message = "\n\nURL: http://localhost/" + Request.Path
+ "\n\nMESSAGE:\n " + Server.GetLastError().Message
+ "\n\nSTACK TRACE:\n" + Server.GetLastError().StackTrace;
// Insert into Event Log
EventLog Log = new EventLog();
Log.Source = LogName;
Log.WriteEntry(Message, EventLogEntryType.Error);
Server.Redirect(Error.htm) // this shall also clear the error
}
</script>

Above is an example of code in Global.asax and the Application_Error method.
The error is logged and then the user is redirected. Unvalidated parameters are being
logged here in the form of Request.Path. Care must be taken not to log or redisplay
unvalidated input from any external source.

===Web.config===
Web.config has a custom errors tag which can be used to handle errors. This is called last and if Page_error or Application_error called and has functionality that functionality shall be executed first. As long as the previous two handling mechanisms do not redirect or clear (Response.Redirect or a Server.ClearError) this shall be called. And you shall be forwarded to the page defined in web.config

<customErrors defaultRedirect="error.html" mode="On|Off|RemoteOnly">
<error statusCode="statuscode" redirect="url"/>
</customErrors>

The “On" directive means that custom errors are enabled. If no defaultRedirect is specified, users see a generic error.
"Off" directive means that custom errors are disabled. This allows display of detailed errors.
"RemoteOnly" specifies that custom errors are shown only to remote clients, and ASP.NET errors are shown to the local host. This is the default.

<customErrors mode="On" defaultRedirect="error.html">
<error statusCode="500" redirect="err500.aspx"/>
<error statusCode="404" redirect="notHere.aspx"/>
<error statusCode="403" redirect="notAuthz.aspx"/>
</customErrors>

== Best Practices for Error Handling ==

===Try & Catch (Java/ .NET)===

Code that might throw exceptions should be in a try block and code that handles exceptions in a catch block.
The catch block is a series of statements beginning with the keyword catch, followed by an exception type and an action to be taken. These are very similar in Java and .NET

Example:

Java Try-Catch:

public class DoStuff {
public static void Main() {
try {
StreamReader sr = File.OpenText("stuff.txt");
Console.WriteLine("Reading line {0}", sr.ReadLine());
}
catch(Exception e) {
Console.WriteLine("An error occurred. Please leave to room”);
logerror(“Error: “, e);
}
}
}

.NET try – catch

public void run() {
while (!stop) {
try {

// Perform work here

} catch (Throwable t) {
// Log the exception and continue
WriteToUser(“An Error has occurred, put the kettle on”);
logger.log(Level.SEVERE, "Unexception exception", t);
}
}
}

In general, it is best practice to catch a specific type of exception rather than use the basic catch(Exception) or catch(Throwable) statement in the case of Java.

===Releasing resources and good housekeeping===

If the language in question has a finally method use it. The finally method is guaranteed to always be called. The finally method can be used to release resources referenced by the method that threw the exception. This is very important. An example would be if a method gained a database connection from a pool of connections and an exception occurred without finally the connection object shall not be returned to the pool for some time (until the timeout). This can lead to pool exhaustion. finally() is called even if no exception is thrown.

try {
System.out.println("Entering try statement");
out = new PrintWriter(new FileWriter("OutFile.txt"));
//Do Stuff….

} catch (Exception e) {
System.err.println("Error occurred!”);

} catch (IOException e) {
System.err.println("Input exception ");

} finally {

if (out != null) {
out.close(); // RELEASE RESOURCES
}
}

A Java example showing finally() being used to release system resources.

===Centralised exception handling (Struts Example)===

Building an infrastructure for consistent error reporting proves more difficult than error handling.
Struts provides the ActionMessages & ActionErrors classes for maintaining a stack of error messages to be reported, which can be used with JSP tags like <html: error> to display these error messages to the user.

To report a different severity of a message in a different manner (like error, warning, or information) the following tasks are required:

* Register, instantiate the errors under the appropriate severity
* Identify these messages and show them in a constant manner.

Struts ActionErrors class makes error handling quite easy:

ActionErrors errors = new ActionErrors()
errors.add("fatal", new ActionError("...."));
errors.add("error", new ActionError("...."));
errors.add("warning", new ActionError("...."));
errors.add("information", new ActionError("...."));
saveErrors(request,errors); // Important to do this

Now we have added the errors we display them by using tags in the HTML page.

<logic:messagePresent property="error">
<html:messages property="error" id="errMsg" >
<bean:write name="errMsg"/>
</html:messages>
</logic:messagePresent >

[[Category:OWASP Code Review Project]]

Reviewing Code for Error Handling

2007-10-02T22:25:12Z

Aldosolis: /* JAVA */

[[OWASP Code Review Guide Table of Contents]]__TOC__

==Error, Exception handling & Logging. ==

An important aspect of secure application development is to prevent information leakage. Error messages give an attacker great insight into the inner workings of an application.

The purpose of reviewing the Error Handling code is to assure the application fails safely under all possible error conditions, expected and unexpected. No sensitive information is presented to the user when an error occurs.

For example SQL injection is much tougher to successfully pull off without some healthy error messages. It lessens the attack footprint and our attacker would have to resort to use “blind SQL injection” which is more difficult and time consuming.

A well-planned error/exception handling strategy is important for three reasons:

# Good error handling does not give an attacker any information which is a means to an end, attacking the application
# A proper centralised error strategy is easier to maintain and reduces the chance of any uncaught errors “Bubbling up” to the front end of an application.
# Information leakage can lead to social engineering exploits.

Some development languages provide checked exceptions which mean that the compiler shall complain if an exception for a particular API call is not caught Java and C# are good examples of this.
Languages like C++ and C do not provide this safety net. Languages with checked exception handling still are prone to information leakage as not all types of error are checked for.

When an exception or error is thrown we also need to log this occurrence. Sometimes this is due to bad development, but it can be the result of an attack or some other service your application relies on failing.

All code paths that can cause an exception to be thrown should check for success in order for the exception not to be thrown.

To avoid a NullPointerException we should check is the object being accessed is not null.

==Generic error messages==

We should use a localized description string in every exception, a friendly error reason such as “System Error – Please try again later”. When the user sees an error message, it will be derived from this description string of the exception that was thrown, and never from the exception class which may contain a stack trace, line number where the error occurred, class name or method name.

Do not expose sensitive information in exception messages. Information such as paths on the local file system is considered privileged information; any system internal information should be hidden from the user. As mentioned before an attacker could use this information to gather private user information from the application or components that make up the app.

Don’t put people’s names or any internal contact information in error messages. Don’t put any “human” information, which would lead to a level of familiarity and a social engineering exploit.

==How to locate the potentially vulnerable code==

===JAVA===

In java we have the concept of an error object, the Exception object.
This lives in the java package java.lang and is derived from the Throwable object
Exceptions are thrown when an abnormal occurrence has occurred. Another object derived from Throwable is the Error object, which is thrown when something more serious occurs.

Information leakage can occur when developers use some exception methods, which ‘bubble’ to the user UI due to a poor error handling strategy.
The methods are as follows:
printStackTrace()
getStackTrace()

Also is important to know that by default the output of these methods is printed in System console, the same as System.out.println(e) where e is an Exception. Be sure to not redirect the outputStream to PrintWriter object of JSP, by convention called "out". Ex. printStackTrace(out);

Also another object to look at is the java.lang.system package:

setErr() and the System.err field.

===.NET===

In .NET a System.Exception object exists. Commonly used child objects such as ApplicationException and SystemException are used.
It is not recommended that you throw or catch a SystemException this is thrown by runtime.

When an error occurs, either the system or the currently executing application reports it by throwing an exception containing information about the error, similar to java. Once thrown, an exception is handled by the application or by the default exception handler.
This Exception object contains similar methods to the java implementation such as:

StackTrace
Source
Message
HelpLink

In .NET we need to look at the error handling strategy from the point of view of global error handling and the handling of unexpected errors. This can be done in many ways and this article is not an exhaustive list.
Firstly an Error Event is thrown when an unhandled exception is thrown. This is part of the TemplateControl class.

http://msdn.microsoft.com/library/default.asp?url=/library/en-us/cpref/html/frlrfSystemWebUITemplateControlClassErrorTopic.asp

==Error handling can be done in three ways in .NET==

*In the web.config file's customErrors section.
*In the global.asax file's Application_Error sub.
*On the aspx or associated codebehind page in the Page_Error sub

The order of error handling events in .NET is as follows:
# On the Page in the Page_Error sub.
# The global.asax Application_Error sub
# The web.config file

It is recommended to look in these areas to understand the error strategy of the application.

==Vulnerable Patterns for Error Handling==

===Page_Error===

Page_Error is page level handling which is run on the server side.
Below is an example but the error information is a little too informative and hence bad practice.

'''FIXME: code formatting'''

<nowiki>

<script language="C#" runat="server">

Sub Page_Error(Source As Object, E As EventArgs)

Dim message As String = "<font face=verdana color=red><h1>" & Request.Url.ToString()& "</h1>" &

"<pre><font color='red'>" & Server.GetLastError().ToString()& "</pre></font>"

Response.Write(message) // display message

End Sub

</script>

</nowiki>

The red text in the example above has a number of issues:
Firstly it redisplays the HTTP request to the user in the form of Request.Url.ToString() Assuming there has been no data validation prior to this point we are vulnerable to cross site scripting attacks!! Secondly the error message and stack trace is displayed to the user using Server.GetLastError().ToString() which divulges internal information regarding the application.

After the Page_Error is called, the Application_Error sub is called:

===Global.asax===

When an error occurs, the Application_Error sub is called. In this method we can
log the error and redirect to another page.

<%@ Import Namespace="System.Diagnostics" %>
<script language="C#" runat="server">
void Application_Error(Object sender, EventArgs e) {
String Message = "\n\nURL: http://localhost/" + Request.Path
+ "\n\nMESSAGE:\n " + Server.GetLastError().Message
+ "\n\nSTACK TRACE:\n" + Server.GetLastError().StackTrace;
// Insert into Event Log
EventLog Log = new EventLog();
Log.Source = LogName;
Log.WriteEntry(Message, EventLogEntryType.Error);
Server.Redirect(Error.htm) // this shall also clear the error
}
</script>

Above is an example of code in Global.asax and the Application_Error method.
The error is logged and then the user is redirected. Unvalidated parameters are being
logged here in the form of Request.Path. Care must be taken not to log or redisplay
unvalidated input from any external source.

===Web.config===
Web.config has a custom errors tag which can be used to handle errors. This is called last and if Page_error or Application_error called and has functionality that functionality shall be executed first. As long as the previous two handling mechanisms do not redirect or clear (Response.Redirect or a Server.ClearError) this shall be called. And you shall be forwarded to the page defined in web.config

<customErrors defaultRedirect="error.html" mode="On|Off|RemoteOnly">
<error statusCode="statuscode" redirect="url"/>
</customErrors>

The “On" directive means that custom errors are enabled. If no defaultRedirect is specified, users see a generic error.
"Off" directive means that custom errors are disabled. This allows display of detailed errors.
"RemoteOnly" specifies that custom errors are shown only to remote clients, and ASP.NET errors are shown to the local host. This is the default.

<customErrors mode="On" defaultRedirect="error.html">
<error statusCode="500" redirect="err500.aspx"/>
<error statusCode="404" redirect="notHere.aspx"/>
<error statusCode="403" redirect="notAuthz.aspx"/>
</customErrors>

== Best Practices for Error Handling ==

===Try & Catch (Java/ .NET)===

Code that might throw exceptions should be in a try block and code that handles exceptions in a catch block.
The catch block is a series of statements beginning with the keyword catch, followed by an exception type and an action to be taken. These are very similar in Java and .NET

Example:

Java Try-Catch:

public class DoStuff {
public static void Main() {
try {
StreamReader sr = File.OpenText("stuff.txt");
Console.WriteLine("Reading line {0}", sr.ReadLine());
}
catch(Exception e) {
Console.WriteLine("An error occurred. Please leave to room”);
logerror(“Error: “, e);
}
}
}

.NET try – catch

public void run() {
while (!stop) {
try {

// Perform work here

} catch (Throwable t) {
// Log the exception and continue
WriteToUser(“An Error has occurred, put the kettle on”);
logger.log(Level.SEVERE, "Unexception exception", t);
}
}
}

In general, it is best practice to catch a specific type of exception rather than use the basic catch(Exception) or catch(Throwable) statement in the case of Java.

===Releasing resources and good housekeeping===

If the language in question has a finally method use it. The finally method is guaranteed to always be called. The finally method can be used to release resources referenced by the method that threw the exception. This is very important. An example would be if a method gained a database connection from a pool of connections and an exception occurred without finally the connection object shall not be returned to the pool for some time (until the timeout). This can lead to pool exhaustion. finally() is called even if no exception is thrown.

try {
System.out.println("Entering try statement");
out = new PrintWriter(new FileWriter("OutFile.txt"));
//Do Stuff….

} catch (Exception e) {
System.err.println("Error occurred!”);

} catch (IOException e) {
System.err.println("Input exception ");

} finally {

if (out != null) {
out.close(); // RELEASE RESOURCES
}
}

A Java example showing finally() being used to release system resources.

===Centralised exception handling (Struts Example)===

Building an infrastructure for consistent error reporting proves more difficult than error handling.
Struts provides the ActionMessages & ActionErrors classes for maintaining a stack of error messages to be reported, which can be used with JSP tags like <html: error> to display these error messages to the user.

To report a different severity of a message in a different manner (like error, warning, or information) the following tasks are required:

* Register, instantiate the errors under the appropriate severity
* Identify these messages and show them in a constant manner.

Struts ActionErrors class makes error handling quite easy:

ActionErrors errors = new ActionErrors()
errors.add("fatal", new ActionError("...."));
errors.add("error", new ActionError("...."));
errors.add("warning", new ActionError("...."));
errors.add("information", new ActionError("...."));
saveErrors(request,errors); // Important to do this

Now we have added the errors we display them by using tags in the HTML page.

<logic:messagePresent property="error">
<html:messages property="error" id="errMsg" >
<bean:write name="errMsg"/>
</html:messages>
</logic:messagePresent >

[[Category:OWASP Code Review Project]]