Page History

...

The

...

readlink()

...

function

...

reads

...

where

...

a

...

link

...

points

...

to.

...

It

...

makes

...

no

...

effort

...

to

...

null

...

-terminate

...

its

...

second

...

argument,

...

buffer

...

.

...

Instead,

...

it

...

just

...

returns

...

the

...

number

...

of

...

characters

...

it

...

has

...

written.

...

Noncompliant Code Example

If len is equal to sizeof(buf)

...

,

...

the

...

null terminator is written 1 byte past the end of buf:

 terminator will be written one byte past the end of {{buf}}.

{code:bgColor=#FFcccc}
char buf[2561024];
ssize_t len = readlink("/usr/bin/perl", buf, sizeof(buf));
buf[len] = '\0';

An incorrect solution to this problem is to try to make buf large enough that it can always hold the result:

long symlink_max;
size_t bufsize;
char *buf{code}

A simple (but incorrect) solution to this problem is to try to make {{buf}} large enough that it can always hold the result:

{code bgColor=#ffcccc}
char buf[PATH_MAX+1];
ssize_t len;

errno = readlink 0;
symlink_max = pathconf("/usr/bin/perl", buf, sizeof(buf));
buf[len] = '\0';
{code}

This "fix" incorrectly assumes that {{PATH_MAX}} represents the longest possible path for a file in the filesystem. ({{PATH_MAX}} only bounds the longest possible relative path that can be passed to the kernel in a single call.) On most Unix and Linux systems, there is no easily-determined maximum length for a file path, and so the off-by-one buffer overflow risk is still present.

An additional issue is that {{readlink()}} can return {{-1}} if it fails, causing an off-by-one underflow.

h2. Compliant Solution

{code:bgColor=#ccccff}
char buf[256];
ssizet_t len;

if ((len _PC_SYMLINK_MAX);
if (symlink_max == -1) {
  if (errno != 0) {
    /* handle error condition */
  }
  bufsize = 10000;
}
else {
  bufsize = symlink_max+1;
}

buf = (char *)malloc(bufsize);
if (buf == NULL) {
  /* handle error condition */
}

len = readlink("/usr/bin/perl", buf, bufsize);
buf[len] = '\0';

This modification incorrectly assumes that the symbolic link cannot be longer than the value of SYMLINK_MAX returned by pathconf(). However, the value returned by pathconf() is out of date by the time readlink() is called, so the off-by-one buffer-overflow risk is still present because, between the two calls, the location of /usr/bin/perl can change to a file system with a larger SYMLINK_MAX value. Also, if SYMLINK_MAX is indeterminate (that is, if pathconf() returned -1 without setting errno), the code uses an arbitrary large buffer size (10,000) that it hopes will be sufficient, but there is a small chance that readlink() can return exactly this size.

An additional issue is that readlink() can return -1 if it fails, causing an off-by-one underflow.

Compliant Solution

This compliant solution ensures there is no overflow by reading in only sizeof(buf)-1 characters. It also properly checks to see if an error has occurred:

enum { BUFFERSIZE = 1024 };
char buf[BUFFERSIZE];
ssize_t len = readlink("/usr/bin/perl", buf, sizeof(buf)-1));

if (len != -1) {
    buf[len] = '\0';
}
else {
   /* handle error condition */
}
{code}

h2. Risk Analysis

|| Rule || Severity || Likelihood || Remediation Cost || Priority || Level ||
| POS30-C | *1* (low) | *2* (probable) | *2* (medium) | {color:green}{*}P4{*}{color} | {color:green}{*}L3{*}{color} |

h2. References

\[[ilja 06|AA. C References#ilja 06]\]
\[[Open Group 97|AA. C References#Open Group 97]\]
\[[Open Group 04|AA. C References#Open Group 04]\]

Risk Assessment

Failing to properly null-terminate the result of readlink() can result in abnormal program termination and buffer-overflow vulnerabilities.

Automated Detection

Related Vulnerabilities

Search for vulnerabilities resulting from the violation of this rule on the CERT website.

Related Guidelines

Key here (explains table format and definitions)

CERT-CWE Mapping Notes

Key here for mapping notes

CWE-170 and POS30-C

CWE-170 = Union( POS30-C, list) where list =

• Non-null terminated strings fed to functions other than POSIX readlink()

Bibliography

...

Image AddedImage AddedImage Added