[Koha-devel] Crashes on new Opac Recent Searches cookie in older Koha versions

[Paul]

At 12:54 PM 10/12/2013 +1300, Chris Cormack wrote:
>On 12 October 2013 11:16, Paul <paul.a at aandc.org> wrote:
> > At 12:26 PM 10/11/2013 -0700, Galen Charlton wrote:
> >
> > "Name based" v. "port based", "Nasty side effects" and "negative data" 
> raise
> > flags with me. I've just looked up bug 10657 which either blind-sides me
> > with science or baffles me with bull.  "Storable" and references to "
>
>I totally would not have gone there with the inference that Galen was
>talking bull, but hey if that's how you want to go, each to their own.

Please... I was only saying that *I* was confused (blind-sided or baffled) 
by the Subject: of the email "Crashes..." in the context of Bug 10657. The 
Perl references to "crashing" were (I believed, perhaps incorrectly) taken 
care of when I disabled EnableOpacSearchHistory.

The phrases that I quoted (maybe out of context?) did not reassure me.

I still haven't had time to fully sandbox test the changes 
<http://git.koha-community.org/gitweb/?p=koha.git;a=commitdiff;h=eeedee8c98776eb52c039649f94331de0596db3c;hp=aefc58585340508d110094890410edc45edd91ec> 
to C4/Auth.pm, opac/opac-search-history.pl and opac/opac-search.pl for use 
within our stable production 3.8.5 -- nor to consider a full system upgrade 
from 3.8.5.

As you well know, I'm a believer in system stability over a two year 
period, à la LTS, with only security upgrades. In 2014, we'll look into 
whatever the latest Koha release might be. In the meanwhile, I believed 
(again perhaps incorrectly) that by disabling some functionality (OPAC 
search history) we had the well-publicized security concern under control. 
Until I saw "crashing" in the context of Bug 10657 and the mention by 
Marcel de Rooy of two commits not referencing bug numbers ....

I was asking for clarification -- and certainly had no intention whatsoever 
of questioning Galen's integrity. If Galen read it that way, I apologize 
unreservedly to him. Without wanting to excuse myself, please remember that 
I am generally much more comfortable writing in French rather than in English.

Regards - Paul

> >
> > checked for JSON-correctness and is ignored" are
> > meaningless without context.
> >
> > If there really is a security aspect would someone please explain it?
> >
>Or the implication that he was lying.
>
>Of course there was a security aspect, and that's why we did security
>releases for 3.12.x, 3.10.x and 3.8.x.
>If you read the release notes
>  http://koha-community.org/security-release-july-2013/
>
>And you snipped some lines out of context, and complain for them being
>out of context.
>
>Here is the whole thing, for context.
>
>"When EnableOpacSearchHistory system preference is enabled, Koha
>stores recent search history for anonymous OPAC sessions in a cookie
>called KohaOpacRecentSearches.  In particular, it used to use the
>Storable Perl module to serialize the array of hashrefs representing
>the recent searches.
>
>However, the documentation for Storable strongly recommends [1] that
>data to be deserialized *not* come from untrusted sources -- and
>cookies cannot be considered trustworthy, as most web browsers (to say
>nothing of curl) allow the user to modify them.  There is a
>theoretical possibility that a modification to the
>KohaOpacRecentSearches cookie could result in the execution of
>unauthorized code with the privileges of the Apache backend process.
>
>The 29 July 2013 security update resolves the security issuing by
>replacing use of the Storable module with the JSON, which doesn't by
>default serialize blessed references and does not attempt to
>deserialize and execute coderefs.  The payload of the cookie is
>checked for JSON-correctness and is ignored if it doesn't contain a
>valid (double-URI-encoded) JSON object.  In particularly, any old
>Storable-based cookies are silently ignored.
>
>[1] http://perldoc.perl.org/Storable.html#SECURITY-WARNING "
>
>If you read the link that is provided there it says
>
>"Do not accept Storable documents from untrusted sources!
>Some features of Storable can lead to security vulnerabilities if you
>accept Storable documents from untrusted sources. Most obviously, the
>optional (off by default) CODE reference serialization feature allows
>transfer of code to the deserializing process. Furthermore, any
>serialized object will cause Storable to helpfully load the module
>corresponding to the class of the object in the deserializing module.
>For manipulated module names, this can load almost arbitrary code.
>Finally, the deserialized object's destructors will be invoked when
>the objects get destroyed in the deserializing process. Maliciously
>crafted Storable documents may put such objects in the value of a hash
>key that is overridden by another key/value pair in the same hash,
>thus causing immediate destructor execution.
>In a future version of Storable, we intend to provide options to
>disable loading modules for classes and to disable deserializing
>objects altogether. Nonetheless, Storable deserializing documents from
>untrusted sources is expected to have other, yet undiscovered,
>security concerns such as allowing an attacker to cause the
>deserializer to crash hard.
>Therefore, let me repeat: Do not accept Storable documents from
>untrusted sources!
>If your application requires accepting data from untrusted sources,
>you are best off with a less powerful and more-likely safe
>serialization format and implementation. If your data is sufficently
>simple, JSON is a good choice and offers maximum interoperability."
>
>So we were accepting Storable documents from untrusted sources (the
>entire internet) and this bug stops that happening.
>
>I respectfully request you think about the way you phrase your
>questions in the future, so they sound like questions not insults.
>
>Chris

---
Maritime heritage and history, preservation and conservation,
research and education through the written word and the arts.
<http://NavalMarineArchive.com> and <http://UltraMarine.ca>