Re: [OPEN-ILS-DEV] Integrating automated code analysis tools into regular practice?

Dan Scott Thu, 22 Nov 2007 18:22:29 -0800

On 22/11/2007, Scott McKellar <[EMAIL PROTECTED]> wrote:
>
> --- Dan Scott <[EMAIL PROTECTED]> wrote:
>
> > I was pondering a few possibilities for helping to tighten up our
> > code
> > in a more automated fashion today. I thought I would throw the ideas
> > out there to see if there's interest or support for them...
>
> <snip: info about code-scanning tools>
>
> Mod parent up.
>
> I have stumbled across various problems (security vulnerabilities,
> memory leaks, etc.) in the course of doing other things, but I
> haven't been looking for them systematically.  I've probably
> overlooked more than I've found, even if you don't count the code
> that I haven't looked at yet.  No doubt the automated tools can find
> more stuff faster.  It's the difference between working with a pick
> and shovel and working with a backhoe.
>
> Don't forget good old lint.  I haven't used it yet on this project
> because I've been finding enough to keep me busy with the pick-and-
> shovel approach.  However my experience is that lint usually finds
> at least a few forehead-slappers.
>
> My one reservation is about the idea of posting the results on the
> web.  The point is not that I don't want to air our dirty linens
> in public -- it's all open source, after all -- but I wouldn't want
> to erect needless barriers to the code scans.  If it takes ten
> minutes to run the scans, and three hours to update the website, then
> we probably won't run the scans as often as we should.  I'd rather
> have more scans than prettier web pages.
>
> Hence I suggest that any publication of the scan results involve
> minimal work, because the reporting is rudimentary, automated,
> or both.
>
> I note that Coverity's website already publishes defect counts on the
> projects it covers.


Hi Scott:

I wouldn't propose something that would take a lot of work - I'm lazy
that way :)

For tools like rats and Perl::Critic, we could simply run the tools
over the entire tree of OpenSRF and Evergreen, pipe the output into a
file, and post the file to the Web site. If we had an automated build
machine, we could make that a standard part of each build. This is
also where we would store the results of automated regression test
runs, if we, well, had regression tests :)

A moderately sophisticated approach would diff the output between
builds and flag new warnings, to focus attention on regressions or new
code that had been introduced.

A more sophisticated approach would shove each possible violation into
a database row, then let human eyes determine whether the warning was
valid or not; if not, then a brief rationale could be entered and the
warning could automatically be ignored in future runs (with some
degree of fuzziness for line number drift) and future developers would
be able to find a record of why that particular "violation" is
actually quite intentional and necessary.

But I would be happy with just the least sophisticated approach. The
hard part is where things like Perl::Critic flag coding practices that
verge towards style, e.g. coming down hard on the use of unless() and
postfix conditionals ($foo = $bar unless $bar = 0; and the like).  If
there is violent disagreement with cleaning up these "moderately
severe" violations, we can always just turn that particular rule off
in our profile.

Perl::Critic also provides nice summary statistics. Here's an example
of the summary for Evergreen trunk, with only the most severe
violations flagged:

190 files.
1,558 subroutines/methods.
43,568 statements.
55,421 lines of code.

Average McCabe score of subroutines was 4.37.

537 violations.
Violations per line of code was 0.010.

537 severity 5 violations.

21 violations of BuiltinFunctions::ProhibitStringyEval.
48 violations of InputOutput::ProhibitBarewordFileHandles.
47 violations of InputOutput::ProhibitTwoArgOpen.
12 violations of Modules::RequireBarewordIncludes.
6 violations of Modules::RequireFilenameMatchesPackage.
314 violations of Subroutines::ProhibitExplicitReturnUndef.
1 violations of Subroutines::ProhibitSubroutinePrototypes.
7 violations of TestingAndDebugging::ProhibitNoStrict.
47 violations of TestingAndDebugging::RequireUseStrict.
1 violations of ValuesAndExpressions::ProhibitLeadingZeros.
30 violations of Variables::ProhibitConditionalDeclarations.
3 violations of Variables::RequireLexicalLoopIterators.

(I know you're not a Perl hacker, but for those Perl hackers reading
this who haven't been exposed to Perl::Critic, more info on the
particular violations and the rationale for why they are violations
can be found for each violation at
http://search.cpan.org/dist/Perl-Critic/ and in even greater depth in
Damian Conway's "Perl Best Practices").

I have attached the RATS output in HTML format for the Evergreen tree
as well, for anyone interested in what it has to say.

-- 
Dan Scott
Laurentian University

Entries in perl database: 33
Entries in python database: 62
Entries in c database: 334
Entries in php database: 55

Analyzing ./Evergreen/conf/load_ips.pl
Analyzing ./Evergreen/src/support-scripts/eg_gen_overdue.pl
Analyzing ./Evergreen/src/perlmods/Evergreen/Application/Search/AddedContent/ContentCafe.pm
Analyzing ./Evergreen/src/extras/import/create-batgirl-keyfile.pl
Analyzing ./Evergreen/src/extras/import/drain-batgirl-charge.pl
Analyzing ./Evergreen/src/extras/import/lib_spliter.pl
Analyzing ./Evergreen/src/extras/import/drain-batgirl-intransit.pl
Analyzing ./Evergreen/src/extras/import/drain-batgirl-item.pl
Analyzing ./Evergreen/src/extras/import/parse_patron_xml.pl
Analyzing ./Evergreen/src/extras/import/drain-batgirl-cn.pl
Analyzing ./Evergreen/src/extras/import/pre_cat_items.pl
Analyzing ./Evergreen/src/extras/import/import_clean_marc.pl
Analyzing ./Evergreen/src/extras/import/libfixup.pl
Analyzing ./Evergreen/src/extras/import/import_holdings.pl
Analyzing ./Evergreen/src/extras/import/import_legacy_closings.pl
Analyzing ./Evergreen/src/extras/import/recirc_user_loader.pl
Analyzing ./Evergreen/src/extras/import/drain-batgirl-email.pl
Analyzing ./Evergreen/src/extras/import/import_legacy_hoo.pl
Analyzing ./Evergreen/src/extras/import/user_import.pl
Analyzing ./Evergreen/src/extras/import/barcode_lookup_loader.pl
Analyzing ./Evergreen/src/extras/import/object_dumper.pl
Analyzing ./Evergreen/src/extras/import/import_authority.pl
Analyzing ./Evergreen/src/extras/import/piece_count_loader.pl
Analyzing ./Evergreen/src/extras/import/non-real_user_loader.pl
Analyzing ./Evergreen/src/extras/import/create-batgirl-usermap.pl
Analyzing ./Evergreen/src/extras/import/drain-batgirl-bill.pl
Analyzing ./Evergreen/src/extras/import/drain-batgirl-hold.pl
Analyzing ./Evergreen/src/extras/import/renewal_subtract.pl
Analyzing ./Open-ILS/src/python/oils/utils/csedit.py
Analyzing ./Open-ILS/src/python/oils/utils/idl.py
Analyzing ./Open-ILS/src/python/oils/utils/__init__.py
Analyzing ./Open-ILS/src/python/oils/utils/utils.py
Analyzing ./Open-ILS/src/python/oils/__init__.py
Analyzing ./Open-ILS/src/python/oils/system.py
Analyzing ./Open-ILS/src/python/oils/const.py
Analyzing ./Open-ILS/src/support-scripts/test-scripts/in_house_use.pl
Analyzing ./Open-ILS/src/support-scripts/test-scripts/checkout.pl
Analyzing ./Open-ILS/src/support-scripts/test-scripts/copy_locations.pl
Analyzing ./Open-ILS/src/support-scripts/test-scripts/notes.pl
Analyzing ./Open-ILS/src/support-scripts/test-scripts/simple.pl
Analyzing ./Open-ILS/src/support-scripts/test-scripts/circ_rules.pl
Analyzing ./Open-ILS/src/support-scripts/test-scripts/collections.pl
Analyzing ./Open-ILS/src/support-scripts/test-scripts/circ_load.pl
Analyzing ./Open-ILS/src/support-scripts/test-scripts/container.pl
Analyzing ./Open-ILS/src/support-scripts/test-scripts/copy_notes.pl
Analyzing ./Open-ILS/src/support-scripts/test-scripts/offline.pl
Analyzing ./Open-ILS/src/support-scripts/oils_header.pl
Analyzing ./Open-ILS/src/support-scripts/ac_ctl.pl
Analyzing ./Open-ILS/src/support-scripts/fine_generator.pl
Analyzing ./Open-ILS/src/support-scripts/make-mods.pl
Analyzing ./Open-ILS/src/support-scripts/long-overdue-status-update.pl
Analyzing ./Open-ILS/src/support-scripts/offline-blocked-list.pl
Analyzing ./Open-ILS/src/support-scripts/hold_targeter.pl
Analyzing ./Open-ILS/src/support-scripts/settings-tester.pl
Analyzing ./Open-ILS/src/offline/offline-config.pl
Analyzing ./Open-ILS/src/offline/offline.pl
Analyzing ./Open-ILS/src/reporter/clark-kent.pl
Analyzing ./Open-ILS/src/c-apps/oils_idl-core.c
Analyzing ./Open-ILS/src/c-apps/oils_fetch.c
Analyzing ./Open-ILS/src/c-apps/oils_cstore.c
Analyzing ./Open-ILS/src/c-apps/oils_dataloader.c
Analyzing ./Open-ILS/src/c-apps/oils_utils.c
Analyzing ./Open-ILS/src/c-apps/oils_auth.c
Analyzing ./Open-ILS/src/c-apps/oils_event.c
Analyzing ./Open-ILS/src/apachemods/mod_xmlbuilder.c
Analyzing ./Open-ILS/src/apachemods/mod_xmlent.c
Analyzing ./Open-ILS/src/apachemods/fieldmapper_lookup-gen.pl
Analyzing ./Open-ILS/src/apachemods/json_xml.c
Analyzing ./Open-ILS/src/apachemods/mod_rest_gateway.c
Analyzing ./Open-ILS/src/perlmods/OpenILS/Utils/CStoreEditor.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Utils/Editor.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Utils/FlatXML.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Utils/PermitHold.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Utils/Fieldmapper.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Utils/ZClient.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Utils/SpiderMonkey.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Utils/ScriptRunner.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Utils/ModsParser.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Utils/OfflineStore.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Reporter/SQLBuilder.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Reporter/Proxy.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Circ/CopyLocations.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Circ/Transit.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Circ/Circulate.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Circ/Money.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Circ/Holds.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Circ/ScriptBuilder.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Circ/Survey.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Circ/HoldNotify.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Circ/NonCat.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Circ/StatCat.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage/CDBI/permission.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage/CDBI/config.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage/CDBI/money.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage/CDBI/actor.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage/CDBI/authority.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage/CDBI/action.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage/CDBI/asset.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage/CDBI/biblio.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage/CDBI/container.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage/CDBI/metabib.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage/Driver/Pg/storage.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage/Driver/Pg/fts.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage/Driver/Pg/dbi.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage/Driver/Pg/cdbi.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage/Driver/Pg.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage/Publisher/permission.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage/Publisher/config.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage/Publisher/money.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage/Publisher/actor.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage/Publisher/authority.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage/Publisher/action.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage/Publisher/asset.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage/Publisher/biblio.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage/Publisher/container.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage/Publisher/metabib.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage/Publisher.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage/FTS.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage/CDBI.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Search/Authority.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Search/Zips.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Search/Biblio.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Search/Z3950.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Search/AddedContent.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Search/CNBrowse.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Cat/Merge.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Cat/Utils.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Actor/ClosedDates.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Actor/UserGroups.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Actor/Container.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Collections.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Storage.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Search.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Cat.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Actor.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Penalty.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/SuperCat.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Circ.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Proxy.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Ingest.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Auth.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/AppUtils.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application/Reporter.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/WWW/Reporter/transforms.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/WWW/SuperCat/Feed.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/WWW/AddedContent/Syndetic.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/WWW/AddedContent/Amazon.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/WWW/XMLRPCGateway.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/WWW/SuperCat.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/WWW/AddedContent.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/WWW/Exporter.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/WWW/Method.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/WWW/Web.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/WWW/Redirect.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/WWW/Reporter.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/WWW/Proxy.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/SIP/Transaction/Checkin.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/SIP/Transaction/Checkout.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/SIP/Transaction/Renew.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/SIP/Transaction.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/SIP/Msg.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/SIP/Item.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/SIP/Patron.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Template/Plugin/WebSession.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Template/Plugin/Unicode.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Template/Plugin/WebUtils.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Const.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/SIP.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Event.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Perm.pm
Analyzing ./Open-ILS/src/perlmods/OpenILS/Application.pm
Analyzing ./Open-ILS/src/extras/marcdumper/marcdumper.c
Analyzing ./Open-ILS/src/extras/import/marc2are.pl
Analyzing ./Open-ILS/src/extras/import/pg_loader.pl
Analyzing ./Open-ILS/src/extras/import/generate-srfsh-indexer.pl
Analyzing ./Open-ILS/src/extras/import/marc2bre.pl
Analyzing ./Open-ILS/src/extras/import/marcFilterDump.pl
Analyzing ./Open-ILS/src/extras/import/direct_loader.pl
Analyzing ./Open-ILS/src/extras/import/direct_ingest.pl
Analyzing ./Open-ILS/src/extras/import/update_marc.pl
Analyzing ./Open-ILS/src/extras/import/parallel_pg_loader.pl
Analyzing ./Open-ILS/src/extras/org_tree_html_options.pl
Analyzing ./Open-ILS/src/extras/fieldmapper_IDL.pl
Analyzing ./Open-ILS/src/extras/Perl2REST.pl
Analyzing ./Open-ILS/src/extras/oils_requestor.c
Analyzing ./Open-ILS/src/extras/opensearch.pm
Analyzing ./Open-ILS/src/extras/fieldmapper.pl
Analyzing ./Open-ILS/src/extras/Evergreen.py
Analyzing ./Open-ILS/src/extras/org_tree_js.pl
Analyzing ./Open-ILS/src/cgi-bin/setup.pl
Analyzing ./Open-ILS/admin/ils_admin/setup/ils_data/__init__.py
Analyzing ./Open-ILS/admin/ils_admin/setup/ils_data/models.py
Analyzing ./Open-ILS/admin/ils_admin/setup/__init__.py
Analyzing ./Open-ILS/admin/ils_admin/setup/views.py
Analyzing ./Open-ILS/admin/ils_admin/manage.py
Analyzing ./Open-ILS/admin/ils_admin/__init__.py
Analyzing ./Open-ILS/admin/ils_admin/urls.py
Analyzing ./Open-ILS/xul/staff_client/external/dtd2js.pl

RATS results.

Severity: High
Issue: connect
The second argument specifiying the packed address to bind to, should not be derived from user input. If the address is derived from user input, it is possible for a malicious user to cause the socket to connect to an arbitrary remote address, enabling hijacking of potentially sensitive network data.

./Evergreen/src/extras/import/create-batgirl-keyfile.pl

./Evergreen/src/extras/import/drain-batgirl-charge.pl

./Evergreen/src/extras/import/lib_spliter.pl

./Evergreen/src/extras/import/drain-batgirl-intransit.pl

./Evergreen/src/extras/import/drain-batgirl-item.pl

./Evergreen/src/extras/import/drain-batgirl-cn.pl

./Evergreen/src/extras/import/import_holdings.pl

./Evergreen/src/extras/import/drain-batgirl-email.pl

./Evergreen/src/extras/import/create-batgirl-usermap.pl

./Evergreen/src/extras/import/drain-batgirl-bill.pl

./Evergreen/src/extras/import/drain-batgirl-hold.pl

./Open-ILS/src/support-scripts/offline-blocked-list.pl

./Open-ILS/src/support-scripts/settings-tester.pl

202

./Open-ILS/src/reporter/clark-kent.pl

156

./Open-ILS/src/perlmods/OpenILS/Utils/OfflineStore.pm

./Open-ILS/src/perlmods/OpenILS/Application/Storage/Driver/Pg.pm

100

./Open-ILS/src/perlmods/OpenILS/Application/Storage/Driver/Pg.pm

101

./Open-ILS/src/perlmods/OpenILS/Application/SuperCat.pm

748

./Open-ILS/src/perlmods/OpenILS/Application/Ingest.pm

123

./Open-ILS/src/perlmods/OpenILS/Application/Ingest.pm

287

./Open-ILS/src/perlmods/OpenILS/Application/Ingest.pm

312

./Open-ILS/src/perlmods/OpenILS/Application/AppUtils.pm

./Open-ILS/src/extras/import/marc2bre.pl

Severity: High
Issue: fixed size global buffer
Extra care should be taken to ensure that character arrays that are allocated on the stack are used safely. They are prime targets for buffer overflow attacks.

./Open-ILS/src/c-apps/oils_fetch.c

130

./Open-ILS/src/c-apps/oils_fetch.c

134

./Open-ILS/src/c-apps/oils_cstore.c

2991

./Open-ILS/src/c-apps/oils_cstore.c

3094

./Open-ILS/src/c-apps/oils_utils.c

192

./Open-ILS/src/c-apps/oils_event.c

./Open-ILS/src/apachemods/mod_xmlbuilder.c

247

./Open-ILS/src/apachemods/mod_xmlbuilder.c

302

./Open-ILS/src/apachemods/mod_xmlbuilder.c

395

./Open-ILS/src/apachemods/mod_xmlent.c

244

./Open-ILS/src/apachemods/mod_xmlent.c

388

./Open-ILS/src/apachemods/mod_rest_gateway.c

./Open-ILS/src/extras/marcdumper/marcdumper.c

315

Severity: High
Issue: strcpy
Check to be sure that argument 2 passed to this function call will not copy more data than can be handled, resulting in a buffer overflow.

./Open-ILS/src/apachemods/mod_xmlbuilder.c

249

./Open-ILS/src/extras/marcdumper/marcdumper.c

336

Severity: High
Issue: strcat
Check to be sure that argument 2 passed to this function call will not copy more data than can be handled, resulting in a buffer overflow.

./Open-ILS/src/apachemods/mod_xmlbuilder.c

255

Severity: High
Issue: strncat
Consider using strlcat() instead.

./Open-ILS/src/apachemods/mod_xmlbuilder.c

304

Severity: High
Issue: strncat
Check to be sure that argument 1 passed to this function call will not copy more data than can be handled, resulting in a buffer overflow.

./Open-ILS/src/apachemods/mod_xmlbuilder.c

304

Severity: High
Issue: eval
Using user supplied strings anywhere inside of an eval is extremely dangerous. Unvalidated user input fed into an eval call may allow the user to execute arbitrary perl code. Avoid ever passing user supplied strings into eval.

./Open-ILS/src/perlmods/OpenILS/Utils/SpiderMonkey.pm

339

./Open-ILS/src/perlmods/OpenILS/Utils/SpiderMonkey.pm

350

./Open-ILS/src/perlmods/OpenILS/Utils/SpiderMonkey.pm

366

./Open-ILS/src/perlmods/OpenILS/Utils/SpiderMonkey.pm

374

./Open-ILS/src/perlmods/OpenILS/Utils/SpiderMonkey.pm

380

./Open-ILS/src/perlmods/OpenILS/Utils/SpiderMonkey.pm

386

./Open-ILS/src/perlmods/OpenILS/Utils/ScriptRunner.pm

169

./Open-ILS/src/perlmods/OpenILS/Utils/ScriptRunner.pm

177

./Open-ILS/src/perlmods/OpenILS/Utils/ScriptRunner.pm

537

./Open-ILS/src/perlmods/OpenILS/Utils/ScriptRunner.pm

548

./Open-ILS/src/perlmods/OpenILS/Utils/ScriptRunner.pm

564

./Open-ILS/src/perlmods/OpenILS/Utils/ScriptRunner.pm

572

./Open-ILS/src/perlmods/OpenILS/Utils/ScriptRunner.pm

578

./Open-ILS/src/perlmods/OpenILS/Utils/ScriptRunner.pm

584

Severity: High
Issue: getopt
Truncate all input strings to a reasonable length before passing them to this function

./Open-ILS/src/extras/oils_requestor.c

Severity: High
Issue: compile
Argument 1 to this function call should be checked to ensure that it does not come from an untrusted source without first verifying that it contains nothing dangerous.

./Open-ILS/src/extras/Evergreen.py

Severity: Medium
Issue: open
The filename argument of open should be carefully checked if it is being created with any user-supplied string as a compontent of it. Strings should be checked for occurences of path backtracking/relative path components (../ as an example), or nulls, which may cause the underlying C call to interpret the filename to open differently than expected. It is also important to make sure that the final filename does not end in a "|", as this will cause the path to be executed.

./Evergreen/conf/load_ips.pl

./Evergreen/src/support-scripts/eg_gen_overdue.pl

./Open-ILS/src/support-scripts/test-scripts/circ_load.pl

./Open-ILS/src/support-scripts/fine_generator.pl

./Open-ILS/src/support-scripts/hold_targeter.pl

./Open-ILS/src/support-scripts/settings-tester.pl

275

./Open-ILS/src/support-scripts/settings-tester.pl

280

./Open-ILS/src/offline/offline.pl

216

./Open-ILS/src/offline/offline.pl

417

./Open-ILS/src/offline/offline.pl

498

./Open-ILS/src/reporter/clark-kent.pl

535

./Open-ILS/src/reporter/clark-kent.pl

659

./Open-ILS/src/reporter/clark-kent.pl

763

./Open-ILS/src/apachemods/fieldmapper_lookup-gen.pl

./Open-ILS/src/perlmods/OpenILS/Utils/SpiderMonkey.pm

./Open-ILS/src/perlmods/OpenILS/Utils/ScriptRunner.pm

153

./Open-ILS/src/perlmods/OpenILS/Application/Circ/HoldNotify.pm

303

./Open-ILS/src/perlmods/OpenILS/Application/Search/Zips.pm

./Open-ILS/src/perlmods/OpenILS/Application/Cat.pm

./Open-ILS/src/perlmods/OpenILS/WWW/Redirect.pm

Severity: Medium
Issue: chdir
When using this function, it is important to be sure that the string being passed in does not contain relative path elements (../ for example), or a null, which may cause underlying C calls to behave in ways you do not expect. This is especially important if the string is in any way constructed from a user supplied value.

./Open-ILS/src/support-scripts/settings-tester.pl

262

Severity: Medium
Issue: mkdir
When using this function, it is important to be sure that the string being passed in does not contain relative path elements (../ for example), or a null, which may cause underlying C calls to behave in ways you do not expect. This is especially important if the string is in any way constructed from a user supplied value.

./Open-ILS/src/support-scripts/settings-tester.pl

266

./Open-ILS/src/reporter/clark-kent.pl

192

./Open-ILS/src/reporter/clark-kent.pl

193

./Open-ILS/src/reporter/clark-kent.pl

194

./Open-ILS/src/reporter/clark-kent.pl

195

Severity: Medium
Issue: system
When using system, it is important to be sure that the string being used does not contain relative path elements (../ for example), or a null, which may cause underlying C calls to behave strangely. It is also imperative to insure the string has no characters that may be interpreted by the shell, possibly allowing arbitrary commands to be run

./Open-ILS/src/support-scripts/settings-tester.pl

269

./Open-ILS/src/support-scripts/settings-tester.pl

272

./Open-ILS/src/support-scripts/settings-tester.pl

285

./Open-ILS/src/support-scripts/settings-tester.pl

288

Severity: Medium
Issue: rand
Standard random number generators should not be used to generate randomness used for security reasons. For security sensitive randomnes s a crytographic randomness generator that provides sufficient entropy should be used.

./Open-ILS/src/offline/offline.pl

140

./Open-ILS/src/perlmods/OpenILS/Application/Circ/Circulate.pm

438

./Open-ILS/src/perlmods/OpenILS/Application/Circ/Survey.pm

367

./Open-ILS/src/perlmods/OpenILS/Application/Circ/Survey.pm

389

./Open-ILS/src/perlmods/OpenILS/Application/Storage/Driver/Pg.pm

124

./Open-ILS/src/perlmods/OpenILS/Application/Storage/Publisher/action.pm

1285

./Open-ILS/src/perlmods/OpenILS/Application/Storage/Publisher/action.pm

1299

./Open-ILS/src/perlmods/OpenILS/Application/Storage/CDBI.pm

267

./Open-ILS/src/perlmods/OpenILS/Application/Auth.pm

103

./Open-ILS/src/perlmods/OpenILS/Application/Auth.pm

184