bug in xaraya PHP session save handler

Hi Xaraya Development team,

We've discovered a pretty nasty bug in the xarSession.php
piece of Xaraya.  As far as we can tell, this bug is still
present in the latest release (Xaraya 1.1.2).  Because we
are unable to create a new bug on Xaraya Bugzilla, we
thought we would email the development list in hopes of
someone higher up adding this issue to Bugzilla.  The
effects of this bug, although not common, are data loss and
possible data corruption.  On our heavily used Xaraya site,
http://library.calvin.edu, we've experience this bug on a daily basis, generating
mounting frustration on the part of our editors as well as
lost time and lost work.  Now having invested over 16 man
hours into debugging this problem, we're hoping others can
benefit from our troubleshooting.  Read on for the details. 


Summary
The Xaraya session handling functions provided in
xarSession.php, namely xarSession__phpRead and
xarSession__phpWrite , do not address a fundamental feature
provided in the default file-based PHP session handler:
session locking.  Without session locking code in place, it
is possible for a browser to make simultaneous requests
under the same PHP session causing a race condition between
these multiple requests, possibly resulting in overwriting
of the session data in the database ( the xar_vars BLOB
column in the xar_session_info table).

Practical Applications
We first noticed this bug manifesting itself as the stock
Xaraya error "this operation cannot be performed under
the current circumstances" when some forms were being
submitted, but usually only in FireFox (although it can be
reproduced in IE).  Editors were creating content they had
composed in the WYSIWYG editor, only to submit the page and
be greeted by this fatal error message and consequentially
losing their content edits. 

Technical Details
Having observed that this problem was primarily related to
FireFox, we eventually discovered that a major trigger for
this bug was FireFox's auto-fetching of RSS feeds linked to
in the requested document's rss LINK tag in its HEAD
section.  IE may also eventually behave this way (if it
cannot already be configured to do so) but these two
simultaneous requests (one for RSS and one for the page)
generated a race condition which resulted in close to a 25%
chance of a data-los condition given moderate load on our
server  A similar "failure" condition could be
created in FireFox or IE when an editor used tabs and/or
multiple windows to access multiple data-editing forms
across our site.  Typically the two simultaneous requests
were 1) an RSS feed and 2) a page load.  The RSS feed
request usually finished processing faster than the page
load, but on the occasions when a page finished processing
faster than the feed, the session data ($_SESSION)
associated with the RSS request actually overwrote the
session data just stored by the page request in the
database.  Why is this session data important?  The Xaraya
functions xarSecGenAuthKey() and xarSecConfirmAuthKey() rely
on session variables to ensure that create/edit/delete
operations are not committed without first going through the
proper "page" paths.  If the authkey (stored in
$_SESSION) is lost by another request overwriting the
session data (as was the case), then the editor's form
submission to create/edit/delete content would be be
interpreted by Xaraya as unauthorized, and the content would
be lost.  The end result for our editors was a frequent
data-loss problem, whose lacke of reproducibility was
influenced by server load, browser type, and the
"heaviness" of a particular page load. 

A Solution
After researching the problem on the Internet, we found a
well-written article that addresses PHP session locking in
the context of simultaneous AJAX requests.    http://thwartedefforts.org/2006
/11/11/race-conditions-with-ajax-and-php-sessions/ The
solution we came up with, per the article's suggestion, was
to implement session locking through modifying
xarSession__phpRead and xarSession__phpWrite to use the
MySQL locking features GET_LOCK() and RELEASE_LOCK() (see http://dev.mysql.com/doc/refman/4.1/en/miscel
laneous-functions.html ).

In xarSession_phpRead we added:
    // Prevent simultaneous session reads
    $dbconn->Execute("SELECT
GET_LOCK('XARSESSIONLOCK',60)");

In xarSession_phpWrite we added: 
    // Prevent simultaneous session reads
    $dbconn->Execute("SELECT
RELEASE_LOCK('XARSESSIONLOCK')"); 

Our modified xarSession.php file is attached to this email. 
Unfortunately this solution is dependent upon the locking
MySQL features mentioned above and is not database platform
independent, although it is a start.  We were hoping there
would be some similar ADODB version of this locking behavior
that would make the code platform independent. 

If you have any questions pertaining to this bug, please
email us.  We'd be happy to help explain the problem in more
detail.  We'd like to see this bug placed in Bugzilla so
that it can eventually be addressed.  Please review our
improvements to xarSecGenAuthKey() and
xarSecConfirmAuthKey() in xarSecurity.php as well.

Thank you for your time,

Chris Hirt and Dan Wells
Programmers
Hekman Library - Calvin College
Grand Rapids, MI USA
_______________________________________________
Xaraya_devel mailing list
Xaraya_develxaraya.com
http:
//xaraya.com/mailman/listinfo/xaraya_devel

Thanks for spending the time to post this Chris.

Is this actually two different and distinct problems:

1. The authorization key being 'consumed' through an RSS
feed. The auth key 
is created when a form is being presented, and is consumed
when the form is 
submitted. If any further auth keys are created in the
meantime for that 
session, then the submitted form will be rejected.

2. Race conditions where several pages are being delivered
to the same 
browser session at the same time. I can imagine AJAX
functions could well do 
that.

To fix the first problem, we need to determine why the RSS
feed is pulling 
out a new auth key in the first place. Since there are no
forms in the RSS 
feed, it should not do that - check the module generating
that feed to see 
exactly what it is doing. In fact, I'm wondering whether the
auth key 
generator should check the theme and automatically skip its
function if the 
theme is RSS?

The second problem is more serious, and locking does seem to
be the only way 
to solve it. Some further thought on how to make it more
portable is needed 
though (my cursory glance doesn't do your analysis justice,
so perhaps 
anyone who understands this better can chip in?)

-- Jason


"Chris Hirt" <chirtcalvin.edu> wrote in
message 
news:45CC3FD9.3405.003D.0calvin.edu...
Hi Xaraya Development team,

We've discovered a pretty nasty bug in the xarSession.php
piece of Xaraya. 
As far as we can tell, this bug is still present in the
latest release 
(Xaraya 1.1.2).  Because we are unable to create a new bug
on Xaraya 
Bugzilla, we thought we would email the development list in
hopes of someone 
higher up adding this issue to Bugzilla.  The effects of
this bug, although 
not common, are data loss and possible data corruption.  On
our heavily used 
Xaraya site, http://library.calvin.edu, we've experience this bug on a daily 
basis, generating mounting frustration on the part of our
editors as well as 
lost time and lost work.  Now having invested over 16 man
hours into 
debugging this problem, we're hoping others can benefit from
our 
troubleshooting.  Read on for the details.

Summary
The Xaraya session handling functions provided in
xarSession.php, namely 
xarSession__phpRead and xarSession__phpWrite , do not
address a fundamental 
feature provided in the default file-based PHP session
handler: session 
locking.  Without session locking code in place, it is
possible for a 
browser to make simultaneous requests under the same PHP
session causing a 
race condition between these multiple requests, possibly
resulting in 
overwriting of the session data in the database ( the
xar_vars BLOB column 
in the xar_session_info table).

Practical Applications
We first noticed this bug manifesting itself as the stock
Xaraya error "this 
operation cannot be performed under the current
circumstances" when some 
forms were being submitted, but usually only in FireFox
(although it can be 
reproduced in IE).  Editors were creating content they had
composed in the 
WYSIWYG editor, only to submit the page and be greeted by
this fatal error 
message and consequentially losing their content edits.

Technical Details
Having observed that this problem was primarily related to
FireFox, we 
eventually discovered that a major trigger for this bug was
FireFox's 
auto-fetching of RSS feeds linked to in the requested
document's rss LINK 
tag in its HEAD section.  IE may also eventually behave this
way (if it 
cannot already be configured to do so) but these two
simultaneous requests 
(one for RSS and one for the page) generated a race
condition which resulted 
in close to a 25% chance of a data-los condition given
moderate load on our 
server  A similar "failure" condition could be
created in FireFox or IE when 
an editor used tabs and/or multiple windows to access
multiple data-editing 
forms across our site.  Typically the two simultaneous
requests were 1) an 
RSS feed and 2) a page load.  The RSS feed request usually
finished 
processing faster than the page load, but on the occasions
when a page 
finished processing faster than the feed, the session data
($_SESSION) 
associated with the RSS request actually overwrote the
session data just 
stored by the page request in the database.  Why is this
session data 
important?  The Xaraya functions xarSecGenAuthKey() and 
xarSecConfirmAuthKey() rely on session variables to ensure
that 
create/edit/delete operations are not committed without
first going through 
the proper "page" paths.  If the authkey (stored
in $_SESSION) is lost by 
another request overwriting the session data (as was the
case), then the 
editor's form submission to create/edit/delete content would
be be 
interpreted by Xaraya as unauthorized, and the content would
be lost.  The 
end result for our editors was a frequent data-loss problem,
whose lacke of 
reproducibility was influenced by server load, browser type,
and the 
"heaviness" of a particular page load.

A Solution
After researching the problem on the Internet, we found a
well-written 
article that addresses PHP session locking in the context of
simultaneous 
AJAX requests. 
http://thwartedefforts.org/2006
/11/11/race-conditions-with-ajax-and-php-sessions/ 
The solution we came up with, per the article's suggestion,
was to implement 
session locking through modifying xarSession__phpRead and 
xarSession__phpWrite to use the MySQL locking features
GET_LOCK() and 
RELEASE_LOCK() (see 
http://dev.mysql.com/doc/refman/4.1/en/miscel
laneous-functions.html ).

In xarSession_phpRead we added:
    // Prevent simultaneous session reads
    $dbconn->Execute("SELECT
GET_LOCK('XARSESSIONLOCK',60)");

In xarSession_phpWrite we added:
    // Prevent simultaneous session reads
    $dbconn->Execute("SELECT
RELEASE_LOCK('XARSESSIONLOCK')");

Our modified xarSession.php file is attached to this email. 
Unfortunately 
this solution is dependent upon the locking MySQL features
mentioned above 
and is not database platform independent, although it is a
start.  We were 
hoping there would be some similar ADODB version of this
locking behavior 
that would make the code platform independent.

If you have any questions pertaining to this bug, please
email us.  We'd be 
happy to help explain the problem in more detail.  We'd like
to see this bug 
placed in Bugzilla so that it can eventually be addressed. 
Please review 
our improvements to xarSecGenAuthKey() and
xarSecConfirmAuthKey() in 
xarSecurity.php as well.

Thank you for your time,

Chris Hirt and Dan Wells
Programmers
Hekman Library - Calvin College
Grand Rapids, MI USA 


_______________________________________________
Xaraya_devel mailing list
Xaraya_develxaraya.com
http:
//xaraya.com/mailman/listinfo/xaraya_devel

Chris Hirt wrote:
 > We've discovered a pretty nasty bug in the
xarSession.php piece of 
Xaraya.

Thanks Chris and Dan, we can always count on you guys to
come up with 
something interesting 

I distill about the same things as Jason from your message.
Similar things i have indeed found in our own projects too.

1. auth key consuming in general
--------------------------------
As described by chris and jason, the auth key is basically
only 
relevant when there are forms involved, or, more accurate,
when the 
code path goes through a function which *may* involve a form
and 
generated an auth key to be able to validate the sequence. I
know 
there are some places in the code which need a review on
this, i.e. 
where auth keys are generated unwanted.

One such path is the automaticly available 'rss link' on
every page 
(stick &theme=rss at the end for one). The RSS is never
used in 99% of 
the cases obviously, but the code still goes thru the same
path as if 
it were the normal view, so a key may be generated during
the display 
of a normal page which contains a form. This could explain
the 
prefetch of firefox problem. In this particular example
(rss) it would 
  suffice to either suppress the rss link header generation
for that 
page or change it. The bigger problem is of course the code
path going 
through generating the key twice.

These things are unavoidable other than thinking carefully
where to 
call genauthkey and making sure that what we generate
automatically 
(for example in the meta block) actually makes sense all the
time (rss 
link in admin pages does not, for example)

Every situation where we offer either automatically
generated (like 
with the meta block) or manually entered, alternative views
on the 
same page, the above applies and needs to be reviewed for
this problem.

Typically the improvements are trivial.
(i'm explicitly avoiding the word solution here, cos the
problem as 
such is unavoidable)


2. race conditions / locking / customisations
---------------------------------------------
These problems are more fun 

These typically only surface when you have a heavily used
site or are 
explicitly looking for these things. The average user doesnt
get (hurt 
by) these problems (hopefully)
The second reason that these situations are hard to solve in
a 
generally applicable way, is that they all depend heavily on
the local 
deployment choices. Backend type and configuration, proxies
and other 
local optimisations.

For example, the locking solution chris described is likely
the 
easiest for them, but when using persistent connections the
solution 
might not work anymore and something else has to be figured
out. This 
is indeed typically something that would go in the
middleware layer 
(ADODB)

> happy to help explain the problem in more detail.  We'd
like to see this bug 
> placed in Bugzilla so that it can eventually be
addressed. 
I'll make sure it gets there

Marcel
_______________________________________________
Xaraya_devel mailing list
Xaraya_develxaraya.com
http:
//xaraya.com/mailman/listinfo/xaraya_devel

Hello Jason (and Marcel),

I would like to chime in here because, at least in our case,
problem #1
isn't happening.  We have some custom code which allows us
to have multiple
authkeys per session, but thought it best to treat that as a
separate issue
because we are confident of the problem's cause (again, in
our case).  The
RSS feed never calls xarSecGenAuthKey() or
xarSecConfirmAuthKey().

I know Chris already said it, but it was towards the end of
his quite long
message, so it may help to say it again in a different way
  Here
is an
outline of the race condition we observe:

Given: You currently have stored session information with no
viable auth key
(we'll call this SessionNoAuth

Event 1: Real page request reads SessionNoAuth from DB,
creates auth key
(now we have SessionAuth in memory)

Event 2: RSS page request (or any other near simultaneous
request from same
client) reads same SessionNoAuth from DB, doesn't create or
consume auth key
(now we have SessionNoAuth2 in memory)

Event 3: Real page writes SessionAuth to DB to await the
submission of the
form.

Event 4: RSS page writes SessionNoAuth2 to DB.

Event 5: Submission of SessionAuth form fails, finding only
SessionNoAuth2
in the DB.


I think you both understood this, but I wish to stress that
this situation
does not require AJAX nor any other predictable or
especially rare
condition.  For us, if Event 4 happens before Event 3, all
is well.  If it
doesn't, all is lost.  We actually need to be sure Event 3
comes before
event 2 to cover every situation, and hence the need for a
lock.

I like the thought of using the ADODB layer for this
problem, but I don't
know the best way to proceed.  adLockPessimistic seems like
it may work, but
my understanding is that not all DBs support it and it
sometimes considered
sloppy or lazy.  I think I may tinker around with
adLockOptimitic and see
how it works.

Thanks,
DW

>>> On 2/9/2007 at 4:13 PM, in message
<eqio5h$gdk$1newton.xaraya.com>,
Jason<judgejxaraya.com> wrote:
> Thanks for spending the time to post this Chris.
> 
> Is this actually two different and distinct problems:
> 
> 1. The authorization key being 'consumed' through an
RSS feed. The auth 
> key 
> is created when a form is being presented, and is
consumed when the form 
> is 
> submitted. If any further auth keys are created in the
meantime for that 
> 
> session, then the submitted form will be rejected.
> 
> 2. Race conditions where several pages are being
delivered to the same 
> browser session at the same time. I can imagine AJAX
functions could 
> well do 
> that.
> 
> To fix the first problem, we need to determine why the
RSS feed is 
> pulling 
> out a new auth key in the first place. Since there are
no forms in the 
> RSS 
> feed, it should not do that - check the module
generating that feed to 
> see 
> exactly what it is doing. In fact, I'm wondering
whether the auth key 
> generator should check the theme and automatically skip
its function if 
> the 
> theme is RSS?
> 
> The second problem is more serious, and locking does
seem to be the only 
> way 
> to solve it. Some further thought on how to make it
more portable is 
> needed 
> though (my cursory glance doesn't do your analysis
justice, so perhaps 
> anyone who understands this better can chip in?)
> 
> -- Jason
> 
> 
> "Chris Hirt" <chirtcalvin.edu> wrote in
message 
> news:45CC3FD9.3405.003D.0calvin.edu...
> Hi Xaraya Development team,
> 
> We've discovered a pretty nasty bug in the
xarSession.php piece of 
> Xaraya. 
> As far as we can tell, this bug is still present in the
latest release 
> (Xaraya 1.1.2).  Because we are unable to create a new
bug on Xaraya 
> Bugzilla, we thought we would email the development
list in hopes of 
> someone 
> higher up adding this issue to Bugzilla.  The effects
of this bug, 
> although 
> not common, are data loss and possible data corruption.
 On our heavily 
> used 
> Xaraya site, http://library.calvin.edu, we've experience this bug on a 
> daily 
> basis, generating mounting frustration on the part of
our editors as 
> well as 
> lost time and lost work.  Now having invested over 16
man hours into 
> debugging this problem, we're hoping others can benefit
from our 
> troubleshooting.  Read on for the details.
> 
> Summary
> The Xaraya session handling functions provided in
xarSession.php, namely 
> 
> xarSession__phpRead and xarSession__phpWrite , do not
address a 
> fundamental 
> feature provided in the default file-based PHP session
handler: session 
> locking.  Without session locking code in place, it is
possible for a 
> browser to make simultaneous requests under the same
PHP session causing 
> a 
> race condition between these multiple requests,
possibly resulting in 
> overwriting of the session data in the database ( the
xar_vars BLOB 
> column 
> in the xar_session_info table).
> 
> Practical Applications
> We first noticed this bug manifesting itself as the
stock Xaraya error 
> "this 
> operation cannot be performed under the current
circumstances" when some 
> 
> forms were being submitted, but usually only in FireFox
(although it can 
> be 
> reproduced in IE).  Editors were creating content they
had composed in 
> the 
> WYSIWYG editor, only to submit the page and be greeted
by this fatal 
> error 
> message and consequentially losing their content
edits.
> 
> Technical Details
> Having observed that this problem was primarily related
to FireFox, we 
> eventually discovered that a major trigger for this bug
was FireFox's 
> auto-fetching of RSS feeds linked to in the requested
document's rss LINK

> 
> tag in its HEAD section.  IE may also eventually behave
this way (if it 
> cannot already be configured to do so) but these two
simultaneous 
> requests 
> (one for RSS and one for the page) generated a race
condition which 
> resulted 
> in close to a 25% chance of a data-los condition given
moderate load on 
> our 
> server  A similar "failure" condition could
be created in FireFox or IE 
> when 
> an editor used tabs and/or multiple windows to access
multiple 
> data-editing 
> forms across our site.  Typically the two simultaneous
requests were 1) 
> an 
> RSS feed and 2) a page load.  The RSS feed request
usually finished 
> processing faster than the page load, but on the
occasions when a page 
> finished processing faster than the feed, the session
data ($_SESSION) 
> associated with the RSS request actually overwrote the
session data just 
> 
> stored by the page request in the database.  Why is
this session data 
> important?  The Xaraya functions xarSecGenAuthKey() and

> xarSecConfirmAuthKey() rely on session variables to
ensure that 
> create/edit/delete operations are not committed without
first going 
> through 
> the proper "page" paths.  If the authkey
(stored in $_SESSION) is lost 
> by 
> another request overwriting the session data (as was
the case), then the 
> 
> editor's form submission to create/edit/delete content
would be be 
> interpreted by Xaraya as unauthorized, and the content
would be lost.  
> The 
> end result for our editors was a frequent data-loss
problem, whose lacke 
> of 
> reproducibility was influenced by server load, browser
type, and the 
> "heaviness" of a particular page load.
> 
> A Solution
> After researching the problem on the Internet, we found
a well-written 
> article that addresses PHP session locking in the
context of 
> simultaneous 
> AJAX requests. 
>
http://thwartedefforts.org/2006/11/1
1/race-conditions-with-ajax-and-php-sess
io 
> ns/ 
> The solution we came up with, per the article's
suggestion, was to 
> implement 
> session locking through modifying xarSession__phpRead
and 
> xarSession__phpWrite to use the MySQL locking features
GET_LOCK() and 
> RELEASE_LOCK() (see 
> http://dev.mysql.com/doc/refman/4.1/en/miscel
laneous-functions.html ).
> 
> In xarSession_phpRead we added:
>     // Prevent simultaneous session reads
>     $dbconn->Execute("SELECT
GET_LOCK('XARSESSIONLOCK',60)");
> 
> In xarSession_phpWrite we added:
>     // Prevent simultaneous session reads
>     $dbconn->Execute("SELECT
RELEASE_LOCK('XARSESSIONLOCK')");
> 
> Our modified xarSession.php file is attached to this
email.  
> Unfortunately 
> this solution is dependent upon the locking MySQL
features mentioned 
> above 
> and is not database platform independent, although it
is a start.  We 
> were 
> hoping there would be some similar ADODB version of
this locking 
> behavior 
> that would make the code platform independent.
> 
> If you have any questions pertaining to this bug,
please email us.  We'd 
> be 
> happy to help explain the problem in more detail.  We'd
like to see this 
> bug 
> placed in Bugzilla so that it can eventually be
addressed.  Please 
> review 
> our improvements to xarSecGenAuthKey() and
xarSecConfirmAuthKey() in 
> xarSecurity.php as well.
> 
> Thank you for your time,
> 
> Chris Hirt and Dan Wells
> Programmers
> Hekman Library - Calvin College
> Grand Rapids, MI USA 
_______________________________________________
Xaraya_devel mailing list
Xaraya_develxaraya.com
http:
//xaraya.com/mailman/listinfo/xaraya_devel