#10 Improper handling of blank 'sid' field (patch included)

J Phelps

When scripts in OS AllCommerce use the &check_login
function from login.pm, and they are not called with
the "sid" CGI parameter, they end up generating and
trying to use the query:

SELECT browser,custid,mdate FROM sessions WHERE sid=

This causes an error, which OS AllCommerce's
&query_getAll function fails to detect, because at line
1169 of db_layer.pm, it runs DBI::execute without
checking the return value (the RV is stored in $check,
but never actually checked). If an error occurs, it
won't be caught until line 1172, where
DBI::fetchrow_hashref is used, resulting in a cryptic
error message "fetch without execute".

To fix the bugs, apply the attached patch files. The
patch against login.pm.diff causes &check_login to
check if 'sid' is defined in %{$site} at line 106, the
same place where it checks if $site->{'sid'} is
"nonzero" (or more accurately, not less than zero... is
this another bug, or is the comment wrong?). The error
if $site{'sid'} is not defined is the same as if it was
less than zero.

The patch against db_layer.pm causes query_getAll to
die if $check is not defined after the call to
DBI::execute on line 1169. The result is still a "500
Internal Server Error" if DBI::execute fails, but the
error message in the log file will be more useful in
finding the cause of the problem.


  • J Phelps
    J Phelps

    Kill two bugs with one tarball.

  • J Phelps
    J Phelps

    • summary: Improper handling of blank 'sid' field --> Improper handling of blank 'sid' field (patch included)