[SQU] Code audit? (long post)

From: Pete Philips <pete@dont-contact.us>
Date: Wed, 25 Oct 2000 16:39:41 +0100

Hi.

I was wondering if there has ever been a security code audit
performed on Squid to guard against potential exploits such
as buffer overflows, string format vulnerabilities etc?

I'm no expert in code security but a quick grep of the
2.4.DEVEL4 code base reveals:

% pwd
/tmp/squid-2.4.DEVEL4
% find . -name "*.c" -exec egrep strcat\|strcpy {} \; | wc
    161 416 5101

As I said, I'm not an expert, so a ran ITS4 (the C code
auditor from http://www.cigital.com/its4 ) on the src
directory:

% pwd
/tmp/squid-2.4.DEVEL4/src
% its4 *.c

The (rather long) output is included at the end of this mail.

I'd be interested to hear peoples feedback.

Cheers,

Pete.

 ---------------------------------------------------------------
| Pete Philips \|/ |
| Integralis S3 Team O |
| E-mail: pete.philips@integralis.co.uk |
| Phone: +44 118 930 6060 |
| PGP Key: http://www.s3.integralis.co.uk/pgp/pete.pgp |
 ---------------------------------------------------------------

ITS4-1.1.1 output
-----------------

cache_diff.c:137:(Urgent) fprintf
cache_diff.c:151:(Urgent) fprintf
cache_diff.c:153:(Urgent) fprintf
cache_diff.c:155:(Urgent) fprintf
cache_diff.c:163:(Urgent) fprintf
cache_diff.c:189:(Urgent) fprintf
cache_diff.c:194:(Urgent) fprintf
cache_diff.c:245:(Urgent) fprintf
cachemgr.c:559:(Urgent) fprintf
cachemgr.c:646:(Urgent) fprintf
cachemgr.c:662:(Urgent) fprintf
cachemgr.c:671:(Urgent) fprintf
cachemgr.c:672:(Urgent) fprintf
cachemgr.c:685:(Urgent) fprintf
cachemgr.c:690:(Urgent) fprintf
cachemgr.c:694:(Urgent) fprintf
cachemgr.c:697:(Urgent) fprintf
cachemgr.c:700:(Urgent) fprintf
cachemgr.c:703:(Urgent) fprintf
cachemgr.c:709:(Urgent) fprintf
cf_gen.c:329:(Urgent) fprintf
cf_gen.c:364:(Urgent) fprintf
cf_gen.c:371:(Urgent) fprintf
cf_gen.c:379:(Urgent) fprintf
cf_gen.c:384:(Urgent) fprintf
cf_gen.c:390:(Urgent) fprintf
cf_gen.c:392:(Urgent) fprintf
cf_gen.c:395:(Urgent) fprintf
cf_gen.c:399:(Urgent) fprintf
cf_gen.c:401:(Urgent) fprintf
cf_gen.c:402:(Urgent) fprintf
cf_gen.c:415:(Urgent) fprintf
cf_gen.c:422:(Urgent) fprintf
cf_gen.c:426:(Urgent) fprintf
cf_gen.c:431:(Urgent) fprintf
cf_gen.c:433:(Urgent) fprintf
cf_gen.c:436:(Urgent) fprintf
cf_gen.c:438:(Urgent) fprintf
cf_gen.c:455:(Urgent) fprintf
cf_gen.c:461:(Urgent) fprintf
cf_gen.c:463:(Urgent) fprintf
cf_gen.c:469:(Urgent) fprintf
cf_gen.c:474:(Urgent) fprintf
cf_gen.c:478:(Urgent) fprintf
cf_gen.c:486:(Urgent) fprintf
cf_gen.c:497:(Urgent) fprintf
cf_gen.c:505:(Urgent) fprintf
cf_gen.c:507:(Urgent) fprintf
cf_gen.c:511:(Urgent) fprintf
cf_gen.c:513:(Urgent) fprintf
cf_gen.c:524:(Urgent) fprintf
cf_gen.c:532:(Urgent) fprintf
cf_gen.c:533:(Urgent) fprintf
cf_gen.c:535:(Urgent) fprintf
cf_gen.c:537:(Urgent) fprintf
cf_gen.c:577:(Urgent) fprintf
cf_gen.c:579:(Urgent) fprintf
cf_gen.c:580:(Urgent) fprintf
cf_gen.c:582:(Urgent) fprintf
cf_gen.c:583:(Urgent) fprintf
cf_gen.c:586:(Urgent) fprintf
cf_gen.c:605:(Urgent) fprintf
cf_gen.c:606:(Urgent) fprintf
cf_gen.c:610:(Urgent) fprintf
cf_gen.c:617:(Urgent) fprintf
cf_gen.c:619:(Urgent) fprintf
cf_gen.c:622:(Urgent) fprintf
client.c:75:(Urgent) fprintf
client.c:198:(Urgent) fprintf
client.c:237:(Urgent) fprintf
client.c:268:(Urgent) fprintf
client.c:283:(Urgent) fprintf
client.c:296:(Urgent) fprintf
client.c:320:(Urgent) fprintf
client.c:344:(Urgent) fprintf
client.c:407:(Urgent) fprintf
client.c:413:(Urgent) fprintf
client.c:425:(Urgent) fprintf
debug.c:154:(Urgent) fprintf
debug.c:156:(Urgent) fprintf
dnsserver.c:253:(Urgent) fprintf
dnsserver.c:308:(Urgent) fprintf
dnsserver.c:318:(Urgent) fprintf
dnsserver.c:321:(Urgent) fprintf
main.c:108:(Urgent) fprintf
main.c:754:(Urgent) fprintf
main.c:756:(Urgent) fprintf
main.c:760:(Urgent) fprintf
net_db.c:387:(Urgent) fprintf
net_db.c:395:(Urgent) fprintf
net_db.c:396:(Urgent) fprintf
pinger.c:313:(Urgent) fprintf
pinger.c:320:(Urgent) fprintf
pinger.c:407:(Urgent) fprintf
test_cache_digest.c:145:(Urgent) fprintf
test_cache_digest.c:148:(Urgent) fprintf
test_cache_digest.c:159:(Urgent) fprintf
test_cache_digest.c:191:(Urgent) fprintf
test_cache_digest.c:201:(Urgent) fprintf
test_cache_digest.c:269:(Urgent) fprintf
test_cache_digest.c:284:(Urgent) fprintf
test_cache_digest.c:286:(Urgent) fprintf
test_cache_digest.c:295:(Urgent) fprintf
test_cache_digest.c:324:(Urgent) fprintf
test_cache_digest.c:328:(Urgent) fprintf
test_cache_digest.c:335:(Urgent) fprintf
test_cache_digest.c:347:(Urgent) fprintf
test_cache_digest.c:372:(Urgent) fprintf
test_cache_digest.c:496:(Urgent) fprintf
tools.c:91:(Urgent) fprintf
tools.c:92:(Urgent) fprintf
tools.c:93:(Urgent) fprintf
tools.c:106:(Urgent) fprintf
tools.c:108:(Urgent) fprintf
tools.c:116:(Urgent) fprintf
tools.c:118:(Urgent) fprintf
tools.c:120:(Urgent) fprintf
tools.c:122:(Urgent) fprintf
tools.c:124:(Urgent) fprintf
tools.c:126:(Urgent) fprintf
tools.c:128:(Urgent) fprintf
tools.c:131:(Urgent) fprintf
tools.c:134:(Urgent) fprintf
tools.c:137:(Urgent) fprintf
tools.c:139:(Urgent) fprintf
tools.c:141:(Urgent) fprintf
tools.c:143:(Urgent) fprintf
tools.c:145:(Urgent) fprintf
tools.c:147:(Urgent) fprintf
tools.c:219:(Urgent) fprintf
tools.c:223:(Urgent) fprintf
tools.c:225:(Urgent) fprintf
tools.c:233:(Urgent) fprintf
tools.c:235:(Urgent) fprintf
tools.c:237:(Urgent) fprintf
tools.c:318:(Urgent) fprintf
tools.c:320:(Urgent) fprintf
tools.c:322:(Urgent) fprintf
tools.c:585:(Urgent) fprintf
tools.c:596:(Urgent) fprintf
tools.c:597:(Urgent) fprintf
Non-constant format strings can often be attacked.
Use a constant format string.
----------------
cache_diff.c:204:(Urgent) printf
cachemgr.c:221:(Urgent) printf
cachemgr.c:222:(Urgent) printf
cachemgr.c:224:(Urgent) printf
cachemgr.c:225:(Urgent) printf
cachemgr.c:235:(Urgent) printf
cachemgr.c:236:(Urgent) printf
cachemgr.c:237:(Urgent) printf
cachemgr.c:238:(Urgent) printf
cachemgr.c:239:(Urgent) printf
cachemgr.c:240:(Urgent) printf
cachemgr.c:241:(Urgent) printf
cachemgr.c:242:(Urgent) printf
cachemgr.c:243:(Urgent) printf
cachemgr.c:244:(Urgent) printf
cachemgr.c:245:(Urgent) printf
cachemgr.c:246:(Urgent) printf
cachemgr.c:247:(Urgent) printf
cachemgr.c:248:(Urgent) printf
cachemgr.c:249:(Urgent) printf
cachemgr.c:250:(Urgent) printf
cachemgr.c:251:(Urgent) printf
cachemgr.c:252:(Urgent) printf
cachemgr.c:253:(Urgent) printf
cachemgr.c:260:(Urgent) printf
cachemgr.c:261:(Urgent) printf
cachemgr.c:262:(Urgent) printf
cachemgr.c:263:(Urgent) printf
cachemgr.c:438:(Urgent) printf
cachemgr.c:451:(Urgent) printf
cachemgr.c:454:(Urgent) printf
cachemgr.c:455:(Urgent) printf
cachemgr.c:458:(Urgent) printf
cachemgr.c:459:(Urgent) printf
cachemgr.c:484:(Urgent) printf
cachemgr.c:486:(Urgent) printf
cachemgr.c:495:(Urgent) printf
cf_gen.c:165:(Urgent) printf
cf_gen.c:179:(Urgent) printf
cf_gen.c:180:(Urgent) printf
cf_gen.c:206:(Urgent) printf
cf_gen.c:212:(Urgent) printf
cf_gen.c:218:(Urgent) printf
cf_gen.c:230:(Urgent) printf
cf_gen.c:290:(Urgent) printf
dnsserver.c:196:(Urgent) printf
dnsserver.c:217:(Urgent) printf
dnsserver.c:220:(Urgent) printf
dnsserver.c:232:(Urgent) printf
dnsserver.c:235:(Urgent) printf
dnsserver.c:240:(Urgent) printf
dnsserver.c:242:(Urgent) printf
dnsserver.c:325:(Urgent) printf
main.c:217:(Urgent) printf
recv-announce.c:123:(Urgent) printf
recv-announce.c:125:(Urgent) printf
unlinkd.c:63:(Urgent) printf
unlinkd.c:65:(Urgent) printf
Non-constant format strings can often be attacked.
Use a constant format string.
----------------
tools.c:77:(Urgent) snprintf
Non-constant format strings can often be attacked.
Use a constant format string.
----------------
debug.c:88:(Urgent) syslog
main.c:837:(Urgent) syslog
main.c:841:(Urgent) syslog
main.c:867:(Urgent) syslog
main.c:871:(Urgent) syslog
main.c:883:(Urgent) syslog
main.c:887:(Urgent) syslog
main.c:889:(Urgent) syslog
main.c:896:(Urgent) syslog
tools.c:316:(Urgent) syslog
Non-constant format strings can often be attacked.
Use a constant format string.
----------------
tools.c:96:(Urgent) system
Easy to run arbitrary commands through env vars. Also, potential TOCTOU
problems.
Use fork + execve instead.
----------------
MemBuf.c:239:(Urgent) vsnprintf
debug.c:86:(Urgent) vsnprintf
logfile.c:156:(Urgent) vsnprintf
store.c:494:(Urgent) vsnprintf
tools.c:369:(Urgent) vsnprintf
Non-constant format strings can often be attacked.
Use a constant format string.
----------------
main.c:450:(Very Risky) chroot
Potential race condition on: Config.chroot_dir
Points of concern are:
main.c:450: chroot
main.c:651: chroot
main.c:659: chroot
Manipulate file descriptors, not symbolic names, when possible.
----------------
cachemgr.c:574:(Very Risky) getenv
cachemgr.c:586:(Very Risky) getenv
cachemgr.c:590:(Very Risky) getenv
cachemgr.c:604:(Very Risky) getenv
pinger.c:371:(Very Risky) getenv
Often seen in conjunction with buffer overflows, etc.
Remember that env vars can contain arbitrary malicious input. Test accordingly
before use.
----------------
debug.c:215:(Very Risky) rename
Potential race condition on: from
Points of concern are:
debug.c:215: rename
tools.c:885: rename
Manipulate file descriptors, not symbolic names, when possible.
----------------
cf_gen.c:589:(Very Risky) sprintf
cf_gen.c:594:(Very Risky) sprintf
This function is high risk for buffer overflows
Use snprintf if available, or precision specifiers, if available.
----------------
acl.c:426:(Very Risky) sscanf
acl.c:428:(Very Risky) sscanf
acl.c:430:(Very Risky) sscanf
acl.c:432:(Very Risky) sscanf
acl.c:435:(Very Risky) sscanf
acl.c:437:(Very Risky) sscanf
cache_manager.c:112:(Very Risky) sscanf
ftp.c:500:(Very Risky) sscanf
ftp.c:502:(Very Risky) sscanf
ftp.c:504:(Very Risky) sscanf
ftp.c:535:(Very Risky) sscanf
ftp.c:537:(Very Risky) sscanf
gopher.c:207:(Very Risky) sscanf
gopher.c:747:(Very Risky) sscanf
This function is high risk for buffer overflows
Use precision specifiers, or do your own parsing.
----------------
cache_cf.c:1863:(Very Risky) stat
debug.c:200:(Very Risky) stat
Potential race condition on: debug_log_file
Points of concern are:
debug.c:200: stat
debug.c:220: rename
Manipulate file descriptors, not symbolic names, when possible.
----------------
client.c:207:(Very Risky) strcat
client.c:211:(Very Risky) strcat
client.c:215:(Very Risky) strcat
client.c:219:(Very Risky) strcat
client.c:223:(Very Risky) strcat
client.c:230:(Very Risky) strcat
client.c:232:(Very Risky) strcat
client.c:234:(Very Risky) strcat
ftp.c:1007:(Very Risky) strcat
ftp.c:1010:(Very Risky) strcat
ftp.c:1013:(Very Risky) strcat
ftp.c:1017:(Very Risky) strcat
ftp.c:1020:(Very Risky) strcat
ftp.c:1024:(Very Risky) strcat
ftp.c:1027:(Very Risky) strcat
gopher.c:487:(Very Risky) strcat
gopher.c:523:(Very Risky) strcat
gopher.c:547:(Very Risky) strcat
send-announce.c:74:(Very Risky) strcat
send-announce.c:80:(Very Risky) strcat
send-announce.c:83:(Very Risky) strcat
send-announce.c:88:(Very Risky) strcat
This function is high risk for buffer overflows
Use strncat instead.
----------------
client.c:116:(Very Risky) strcpy
client_side.c:2421:(Very Risky) strcpy
ftp.c:956:(Very Risky) strcpy
ftp.c:2550:(Very Risky) strcpy
icmp.c:172:(Very Risky) strcpy
pconn.c:193:(Very Risky) strcpy
pconn.c:221:(Very Risky) strcpy
recv-announce.c:100:(Very Risky) strcpy
url.c:277:(Very Risky) strcpy
url.c:280:(Very Risky) strcpy
url.c:412:(Very Risky) strcpy
useragent.c:76:(Very Risky) strcpy
This function is high risk for buffer overflows
Use strncpy instead.
----------------
store_io.c:96:(Very Risky) unlink
tools.c:97:(Very Risky) unlink
Potential race condition on: filename
Points of concern are:
tools.c:97: unlink
mime.c:291: fopen
tools.c:89: fopen
Manipulate file descriptors, not symbolic names, when possible.
----------------
main.c:422:(Risky) chdir
main.c:437:(Risky) chdir
Can lead to process/file interaction race conditions (TOCTOU problems)
Manipulate file descriptors, not symbolic names, when possible.
----------------
event.c:83:(Risky) drand48
Don't use rand() and friends for security-critical needs.
Use better sources of randomness, like /dev/random (linux) or Yarrow
(windows).
----------------
main.c:789:(Risky) execl
Many potential problems.
Close all fds, clean the environment, set the umask to something good, and reset

uids before calling.
----------------
ipc.c:277:(Risky) execvp
main.c:866:(Risky) execvp
Many potential problems.
Close all fds, clean the environment, set the umask to something good, and reset

uids before calling.
----------------
cachemgr.c:401:(Risky) fdopen
ipc.c:278:(Risky) fdopen
Can be involved in a race condition if you open things after a poor check. For
example, don't check to see if something is not a symbolic link before opening
it. Open it, then check bt querying the resulting object. Don't run tests on
symbolic file names...
Perform all checks AFTER the open, and based on the returned object, not a
symbolic name.
----------------
acl.c:119:(Risky) fopen
cache_cf.c:218:(Risky) fopen
cache_diff.c:135:(Risky) fopen
cf_gen.c:144:(Risky) fopen
cf_gen.c:317:(Risky) fopen
cf_gen.c:339:(Risky) fopen
debug.c:152:(Risky) fopen
dns_internal.c:145:(Risky) fopen
test_cache_digest.c:143:(Risky) fopen
tools.c:588:(Risky) fopen
Can be involved in a race condition if you open things after a poor check. For
example, don't check to see if something is not a symbolic link before opening
it. Open it, then check bt querying the resulting object. Don't run tests on
symbolic file names...
Perform all checks AFTER the open, and based on the returned object, not a
symbolic name.
----------------
client.c:202:(Risky) fstat
comm_select.c:964:(Risky) fstat
errorpage.c:175:(Risky) fstat
mime.c:409:(Risky) fstat
Can lead to process/file interaction race conditions (TOCTOU category C)
Manipulate file descriptors, not symbolic names, when possible.
----------------
client.c:195:(Risky) open
client_side.c:2384:(Risky) open
ipc.c:254:(Risky) open
main.c:844:(Risky) open
main.c:850:(Risky) open
recv-announce.c:105:(Risky) open
store_dir.c:320:(Risky) open
unlinkd.c:53:(Risky) open
Can be involved in a race condition if you open things after a poor check. For
example, don't check to see if something is not a symbolic link before opening
it. Open it, then check bt querying the resulting object. Don't run tests on
symbolic file names...
Perform all checks AFTER the open, and based on the returned object, not a
symbolic name.
----------------
debug.c:182:(Risky) openlog
main.c:835:(Risky) openlog
main.c:863:(Risky) openlog
main.c:870:(Risky) openlog
Can lead to process/file interaction race conditions (TOCTOU category B)
Manipulate file descriptors, not symbolic names, when possible.
----------------
logfile.c:89:(Risky) stat
Can lead to process/file interaction race conditions (TOCTOU category A)
Manipulate file descriptors, not symbolic names, when possible.
----------------
tools.c:87:(Risky) tempnam
Can lead to process/file interaction race conditions (TOCTOU problems)
Manipulate file descriptors, not symbolic names, when possible.
----------------
unlinkd.c:58:(Risky) truncate
Can lead to process/file interaction race conditions (TOCTOU problems)
Manipulate file descriptors, not symbolic names, when possible.
----------------
main.c:599:(Risky) umask
main.c:601:(Risky) umask
tools.c:561:(Risky) umask
tools.c:563:(Risky) umask
Setting a liberal umask can be bad when you exec an untrusted process.
Reset the umask to something sane before execing.
----------------
tools.c:480:(Risky) unlink
unlinkd.c:60:(Risky) unlink
Can lead to process/file interaction race conditions (TOCTOU category A)
Manipulate file descriptors, not symbolic names, when possible.
----------------
client.c:119:(Some risk) getopt
dnsserver.c:280:(Some risk) getopt
main.c:118:(Some risk) getopt
Depending on the lib implementation, can be a buffer overflow problem.
Truncate all str inputs to a reasonable size before calling this.
----------------
client.c:289:(Some risk) read
client.c:300:(Some risk) read
client_side.c:806:(Some risk) read
client_side.c:2460:(Some risk) read
comm.c:535:(Some risk) read
disk.c:355:(Some risk) read
errorpage.c:182:(Some risk) read
ftp.c:877:(Some risk) read
ftp.c:1244:(Some risk) read
gopher.c:614:(Some risk) read
helper.c:290:(Some risk) read
http.c:486:(Some risk) read
ident.c:143:(Some risk) read
ipc.c:187:(Some risk) read
mime.c:433:(Some risk) read
pconn.c:123:(Some risk) read
pump.c:251:(Some risk) read
send-announce.c:92:(Some risk) read
ssl.c:202:(Some risk) read
ssl.c:238:(Some risk) read
store_io.c:82:(Some risk) read
unlinkd.c:117:(Some risk) read
wais.c:106:(Some risk) read
whois.c:92:(Some risk) read
Be careful not to introduce a buffer overflow when using in a loop.
Make sure to check your buffer boundries.

--
To unsubscribe, see http://www.squid-cache.org/mailing-lists.html
Received on Wed Oct 25 2000 - 09:42:14 MDT

This archive was generated by hypermail pre-2.1.9 : Tue Dec 09 2003 - 16:55:57 MST