mmaraya / netkit-ftp Goto Github PK

ftp/cmds.c

uninit_use: Using uninitialized value te[toknum]

Incorrect values could be read from, or even written to, an arbitrary memory location, causing incorrect computations. In domap: Reads an uninitialized pointer or its target (CWE-457)

ftp/cmds.c#L870

leaked_storage: Variable tp going out of scope leaks the storage it points to.

The system resource will not be reclaimed and reused, reducing the future availability of the resource.
In mget: Leak of memory or pointers to system resources (CWE-404)

ftp/ftp.c#L756

var_compare_op: Comparing local to null implies that local might be null.

ftp/ftp.c#L783

var_deref_model: Passing null pointer local to strcmp, which dereferences it.

Either the check against null is unnecessary, or there may be a null pointer dereference.
In help: Pointer is checked against null but then dereferenced anyway (CWE-476)

This new defect was introduced in 52c0978 when attempting to close #17.

ftp/ftp.c#L513

alloc: Allocation of memory which must be freed using pclose.
assign: Assigning: fin = popen(local + 1, "r").

ftp/ftp.c#L547

free: Calling fclose frees fin using fclose but it should have been freed using pclose.

Depending on the difference between the correct and incorrect allocator, there may be a resource leak or memory corruption. In sendrequest: A resource is freed using the wrong deallocator (CWE-762)

Configure program for static code analysis

It's well-known that various FTP clients can make use of the ~/.netrc file, but perhaps less well-known that clients such as curl and wget can use the file not only for FTP but also for HTTP user auth. It would be nice to mention this in netrc(1).

the ftp command read the .netrc file at every login attempt trying to find a matching authentication.
However, this file is shared among many other process, some that now require the usage of authentication token, which can be very long (up to 256 characters).

If one of such entry is present in the .netrc file, ftp will crash with a segmentation fault until the entry is removed from .netrc, even if the entry is not used by ftp.

Any field (machine, login, password) that is longer than 103 characters will trigger the segmentation fault.

How to reproduce:
1- Create a .netrc file in your home directory.
2- Put an entry with any file longer than 103 characters. Ex:
machine ftp.example.org login anonymous password 0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012
3- Try to login to any FTP. You will be able to log in.
4- Increase the password by ONE character. Ex:
machine ftp.example.org login anonymous password 01234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123
5- Try to log in: Segmentation Fault.

ftp/cmds.c#L701
ftp/cmds.c#L703
ftp/cmds.c#L708
ftp/cmds.c#L768

use_invalid: Using argv[2], which points to an out-of-scope variable tmpbuf.

Dereferencing the returned or out-of-scope stack pointer will access an invalid location on the stack after its scope or after the function returns. In get: Pointer to a local stack variable returned or used outside scope (CWE-562)

ftp/cmds.c#L921

negative_return_fn: Function dup(fd) returns a negative number.
var_assign: Assigning: signed variable dupfd = dup.

ftp/cmds.c#L924

negative_returns: dupfd is passed to a parameter that cannot be negative

The negative value may be unexpected by later operations, causing incorrect computations.
In remglob: Negative value can be returned from function is not being checked before being used improperly (CWE-394)

ftp/glob.c#L702

suspicious_sizeof: Passing argument (blklen(v) + 1) * 8UL /* sizeof (char **) */ to function malloc and then casting the return value to char ** is suspicious. In this particular case sizeof (char **) happens to be equal to sizeof (char *), but this is not a portable assumption.
Did you intend to use sizeof (char *) instead of sizeof (char **)?

While the program behaves correctly on the current processor architecture, it relies on potentially non-portable assumptions about type sizes. In copyblk: The sizeof operator is used on a wrong argument that incidentally has the same size (CWE-467)

ftp/cmds.c#L1545

escape_local_addr: Returning, through argv[3], the address of stack variable theacct.

Dereferencing the returned or out-of-scope stack pointer will access an invalid location on the stack after its scope or after the function returns. In user: Pointer to a local stack variable returned or used outside scope (CWE-562)

ftp/cmds.c#1769

fixed_size_dest: You might overrun the 8192 byte fixed-size string lyne by copying lineread without checking the length.

The string operation will write past the end of the fixed-size destination buffer if the source buffer is too large. A source buffer of statically unknown size is copied into a fixed-size destination buffer CWE-120

Is there any way to capture failure in command execution, for example when Login authentication failed ?

Currently, the return code is "0" :(

ftp/cmds.c#L815

tainted_string_return_content: remglob returns tainted string content.
var_assign: Assigning: cp = remglob(argv, proxy), which taints cp.

ftp/cmds.c#L821

var_assign_var: Assigning: tp = cp. Both are now tainted.

ftp/cmds.c#L846

tainted_data_transitive: Call to function pathprotect with tainted argument tp returns tainted data.
var_assign: Assigning: tp = pathprotect(tp), which taints tp.

ftp/cmds.c#L849

tainted_data_transitive: Call to function pipeprotect with tainted argument tp returns tainted data.
var_assign: Assigning: tp = pipeprotect(tp), which taints tp.

ftp/cmds.c#L855

tainted_string: Passing tainted string tp to recvrequest, which cannot accept tainted data.

The string may be incorrectly assumed to not contain certain metacharacters or element names in later operations. In mget: An unscrutinized string from an untrusted source used in a trusted context (CWE-20)

ftp/cmds.c#L1845

check_return: Calling command without checking return value (as is done elsewhere 34 out of 36 times).

If the function returns an error value, the error value may be mistaken for a normal value.
In account: Value returned from a function is not checked for errors before being used (CWE-252)

var_decl: Declaring variable fout without initializer.

ftp/ftp.c#743

uninit_use: Using uninitialized value fout.

ftp/ftp.c#1067

Incorrect values could be read from, or even written to, an arbitrary memory location, causing incorrect computations. In recvrequest: Reads an uninitialized pointer or its target (CWE-457)

ftp/cmds.c

uninit_use: Using uninitialized value tp[toknum]

Incorrect values could be read from, or even written to, an arbitrary memory location, causing incorrect computations. In domap: Reads an uninitialized pointer or its target (CWE-457)

ftp/ftp.c#1543

You might overrun the 4096 byte fixed-size string new by copying local without checking the length. This defect has an elevated risk because the source argument is a parameter of the current function.

The string operation will write past the end of the fixed-size destination buffer if the source buffer is too large. In gunique: A source buffer of statically unknown size is copied into a fixed-size destination buffer (CWE-120).

ftp/ftp.c#L784

fs_check_call: Calling function access to perform check on local

ftp/ftp.c#L876

toctou: Calling function fopen that uses local after a check function. This can cause a time-of-check, time-of-use race condition

An attacker could change the filename's file association or other attributes between the check and use.
In recvrequest: A check occurs on a file's attributes before the file is used in a privileged operation, but things may have changed (CWE-367)

ftp/ftp.c#L910
ftp/ftp.c#L975
ftp/ftp.c#L1049
ftp/ftp.c#L1066

leaked_storage: Variable fout going out of scope leaks the storage it points to.

The system resource will not be reclaimed and reused, reducing the future availability of the resource.
In recvrequest: Leak of memory or pointers to system resources (CWE-404)

ftp/glob.c#L285

var_compare_op: Comparing gpath to null implies that gpath might be null.

ftp/glob.c#L301

var_compare_op: Passing null pointer gpath to Gcat, which dereferences it.

Either the check against null is unnecessary, or there may be a null pointer dereference.
In help: Pointer is checked against null but then dereferenced anyway (CWE-476)

Under OS X 10.6, void *setmode(const char *) from /usr/include/unistd.h:695:7 conflicts with void setmode(void) in https://github.com/mmaraya/netkit-ftp/blob/master/ftp/cmds.c#L432

gcc (Ubuntu 4.8.4-2ubuntu1~14.04.1) 4.8.4

ftp/cmds.c#L1545

warning: ignoring return value of ‘fgets’, declared with attribute warn_unused_result

The software does not check the return value from a method or function, which can prevent it from detecting unexpected states and conditions. (CWE-252)

ftp/ftp.c#L478

var_compare_op: Comparing local to null implies that local might be null.

ftp/ftp.c#L509

var_deref_model: Passing null pointer local to strcmp, which dereferences it.

Either the check against null is unnecessary, or there may be a null pointer dereference.
In help: Pointer is checked against null but then dereferenced anyway (CWE-476)

ftp/cmds.c#331

fixed_size_dest: You might overrun the 32 byte fixed-size string typename by copying p->t_name without checking the length.

The string operation will write past the end of the fixed-size destination buffer if the source buffer is too large. A source buffer of statically unknown size is copied into a fixed-size destination buffer CWE-120

ftp/main.c#L574

var_compare_op: Comparing c->c_name to null implies that c->c_name might be null.

ftp/main.c#L583

var_deref_model: Passing null pointer c->c_name to strlen, which dereferences it.

Either the check against null is unnecessary, or there may be a null pointer dereference.
In help: Pointer is checked against null but then dereferenced anyway (CWE-476)

var_decl: Declaring variable closefunc without initializer.

ftp/ftp.c#744

uninit_use: Using uninitialized value closefunc

ftp/ftp.c#1067

Incorrect values could be read from, or even written to, an arbitrary memory location, causing incorrect computations. In recvrequest: Reads an uninitialized pointer or its target (CWE-457)

ftp/glob.c

fixed_size_dest: You might overrun the 8194 byte fixed-size string restbuf by copying pe + 1 without checking the length.

The string operation will write past the end of the fixed-size destination buffer if the source buffer is too large. A source buffer of statically unknown size is copied into a fixed-size destination buffer CWE-120

ftp/cmds.c#L1657

CID 47538 (#1 of 1): Copy into fixed size buffer (STRING_OVERFLOW)
1. fixed_size_dest: You might overrun the 8192 byte fixed-size string buf by copying initial without checking the length.
2. parameter_as_source: Note: This defect has an elevated risk because the source argument is a parameter of the current function.

The string operation will write past the end of the fixed-size destination buffer if the source buffer is too large. In quote1: A source buffer of statically unknown size is copied into a fixed-size destination buffer CWE-120

The BSD-4-clause license that is in use in netkit-ftp applies only to files that have a copyright by The Regents of the University of California. It is well known that they allow to drop the advertisement clause (3.) from the license, resulting in BSD-3-clause.

Please drop every appearance of the advertisement clause from the source. This is actually necessary to enable binary distribution of the program linked with readline, which is GPL licensed. The advertisement clause is incompatible with the GPL.

ftp/ftp.c#L513

alloc_fn: Storage is returned from allocation function popen.
var_assign: Assigning: fin = storage returned from popen(local + 1, "r").

ftp/ftp.c#L548

leaked_storage: Variable fin going out of scope leaks the storage it points to.

The system resource will not be reclaimed and reused, reducing the future availability of the resource.
In sendrequest: Leak of memory or pointers to system resources (CWE-404)

ftp/glob.c#L719

suspicious_sizeof: Passing argument (cblklen(v) + 1) * 8UL /* sizeof (char **) */ to function malloc and then casting the return value to char ** is suspicious. In this particular case sizeof (char **) happens to be equal to sizeof (char *), but this is not a portable assumption.
Did you intend to use sizeof (char *) instead of sizeof (char **)?

While the program behaves correctly on the current processor architecture, it relies on potentially non-portable assumptions about type sizes. In cloneblk: The sizeof operator is used on a wrong argument that incidentally has the same size (CWE-467)

ftp/ftp.c#L861

alloc_fn: Storage is returned from allocation function dataconn.
var_assign: Assigning: din = storage returned from dataconn("r").

ftp/ftp.c#L1059

leaked_storage: Variable din going out of scope leaks the storage it points to.

The system resource will not be reclaimed and reused, reducing the future availability of the resource.
In recvrequest: Leak of memory or pointers to system resources (CWE-404)

ftp/cmds.c#L2239

check_return: Calling command without checking return value (as is done elsewhere 34 out of 36 times).

If the function returns an error value, the error value may be mistaken for a normal value.
In syst: Value returned from a function is not checked for errors before being used (CWE-252)

ftp/cmds.c

string_size: Passing string namep of unknown size to strcat, which expects a string of a particular size.

The string operation can write past the end of the fixed-size destination buffer if the source buffer is too large. In shell: A source buffer of arbitrarily large size is used where a fixed-size destination buffer is expected (CWE-120)

The configure script produces a mangled MCONFIG file under OS X 10.11

echo 'Generating MCONFIG...'
(
    echo -n '# Generated by configure (confgen version 2) on '
    date
    echo '#'
    echo

    echo "BINDIR=$BINDIR"
    echo "MANDIR=$MANDIR"
    echo "BINMODE=$BINMODE"
    echo "MANMODE=$MANMODE"
    echo "PREFIX=$PREFIX"
    echo "EXECPREFIX=$EXECPREFIX"
    echo "INSTALLROOT=$INSTALLROOT"
    echo "CC=$CC"
    if [ x$CC_WARNINGS != x ]; then
        CFLAGS="$CFLAGS $WARNINGS"
    fi

    echo "CFLAGS=$CFLAGS" | sed 's/= */=/'
    echo "LDFLAGS=$LDFLAGS" | sed 's/= */=/'
    echo "LIBS=$LIBS" | sed 's/= */=/'

    echo "LIBTERMCAP=$LIBTERMCAP"
    echo "USE_GLIBC=$USE_GLIBC"
    echo "USE_READLINE=$USE_READLINE"
) > MCONFIG

produces

-n # Generated by configure (confgen version 2) on 
Mon Dec  7 22:19:27 EST 2015
#

BINDIR=/usr/bin
MANDIR=/usr/man
BINMODE=755
MANMODE=644
PREFIX=/usr
EXECPREFIX=/usr
INSTALLROOT=
CC=gcc
CFLAGS=-O2 -Wall -W -Wpointer-arith -Wbad-function-cast -Wcast-qual -Wstrict-prototypes -Wmissing-prototypes -Wmissing-declarations -Wnested-externs -Winline 
LDFLAGS=
LIBS=
LIBTERMCAP=-lncurses
USE_GLIBC=
USE_READLINE=

ftp/cmds.c#L1481

tainted_string_return_content: getenv returns tainted string content.
var_assign: Assigning: theshell = getenv("SHELL"), which taints the shell.

ftp/cmds.c#L1500

tainted_string: Passing tainted string theshell to execl, which cannot accept tainted data.

The string may be incorrectly assumed to not contain certain metacharacters or element names in later operations. In shell: An unscrutinized string from an untrusted source used in a trusted context (CWE-20)