Do not make any assumptions about the size of environment variables because an adversary might have full control over the environment. If the environment variable needs to be stored, the length of the associated string should be calculated and the storage dynamically allocated (see STR31-C. Guarantee that storage for strings has sufficient space for character data and the null terminator).
Noncompliant Code Example
This noncompliant code example copies the string returned by getenv()
into a fixed-size buffer:
void f() { char path[PATH_MAX]; /* Requires PATH_MAX to be defined */ strcpy(path, getenv("PATH")); /* Use path */ }
Even if your platform assumes that $PATH
is defined, defines PATH_MAX
, and enforces that paths not have more than PATH_MAX
characters, the $PATH
environment variable still is not required to have less than PATH_MAX
chars. And if it has more than PATH_MAX
chars, a buffer overflow will result. Also, if $PATH
is not defined, then strcpy()
will attempt to dereference a null pointer.
Compliant Solution
In this compliant solution, the strlen()
function is used to calculate the size of the string, and the required space is dynamically allocated:
void f() { char *path = NULL; /* Avoid assuming $PATH is defined or has limited length */ const char *temp = getenv("PATH"); if (temp != NULL) { path = (char*) malloc(strlen(temp) + 1); if (path == NULL) { /* Handle error condition */ } else { strcpy(path, temp); } /* Use path */ free(path); } }
Compliant Solution (POSIX or C2x)
In this compliant solution, the strdup()
function is used to dynamically allocate a duplicate of the string:
void f() { char *path = NULL; /* Avoid assuming $PATH is defined or has limited length */ const char *temp = getenv("PATH"); if (temp != NULL) { path = strdup(temp); if (path == NULL) { /* Handle error condition */ } /* Use path */ free(path); } }
Risk Assessment
Making assumptions about the size of an environmental variable can result in a buffer overflow.
Recommendation | Severity | Likelihood | Remediation Cost | Priority | Level |
---|---|---|---|---|---|
ENV01-C | High | Likely | Medium | P18 | L1 |
Automated Detection
Tool | Version | Checker | Description |
---|---|---|---|
CodeSonar | 8.1p0 | LANG.MEM.BO | Buffer overrun |
Compass/ROSE | Can detect violations of the rule by using the same method as STR31-C. Guarantee that storage for strings has sufficient space for character data and the null terminator | ||
Klocwork | 2024.3 | ABV.ANY_SIZE_ARRAY ABV.GENERAL ABV.GENERAL.MULTIDIMENSION ABV.ITERATOR ABV.MEMBER ABV.STACK ABV.TAINTED ABV.UNKNOWN_SIZE ABV.UNICODE.BOUND_MAP ABV.UNICODE.FAILED_MAP ABV.UNICODE.NNTS_MAP ABV.UNICODE.SELF_MAP | |
Parasoft C/C++test | 2023.1 | CERT_C-ENV01-a | Don't use unsafe C functions that do write to range-unchecked buffers |
PC-lint Plus | 1.4 | 669 | Fully supported |
Polyspace Bug Finder | R2024a | Checks for tainted NULL or non-null-terminated string (rec. partially covered) |
Related Vulnerabilities
Search for vulnerabilities resulting from the violation of this rule on the CERT website.
Related Guidelines
MITRE CWE | CWE-119, Improper Restriction of Operations within the Bounds of a Memory Buffer CWE-123, Write-what-where Condition CWE-125, Out-of-bounds Read |
Bibliography
[IEEE Std 1003.1:2013] | Chapter 8, "Environment Variables" |
[Viega 2003] | Section 3.6, "Using Environment Variables Securely" |
11 Comments
David Svoboda
This rule looks totally subsumed by STR31-C. Guarantee that storage for strings has sufficient space for character data and the null terminator.
Martin Sebor
True, although I think it's still valuable if only as a reminder. I'm pretty sure I've seen code like this:
Might this be a better example?
David Svoboda
Yes. Reworked the NCCE/CS to use your sample.
David Svoboda
Oops...accompanying text need to be updated too.
It does seem that several platforms, including Linux and OSX allow paths to be constructed that are longer than PATH_MAX. While this is an issue, we shouldn't deal with it here (maybe set aside for the POSIX section.) So we focus on that fact that an attacker can set $PATH to anything they wish, incl. a string longer than PATH_MAX.
Martin Sebor
Not only that, an implementation that doesn't impose a restriction on the length of a pathname need not define
PATH_MAX
at all. (When it is defined, it gives the maximum length of a pathname that a system call is able to process at a time. Pathnames that are longer than that are valid, they just cannot be processed in a single call but may require intermediate steps.)David Svoboda
True, PATH_MAX need not be defined. If it isn't, then the NCCE won't compile. Added the assumption that PATH_MAX is at least defined
Martin Sebor
In my experience, assuming that
PATH_MAX
and other similar resource limit constants (or optional types) are necessarily defined, expand to values suitable for use in array declarations, or represent parameters that cannot change at runtime is a sufficiently common mistake to warrant a guideline warning users against such assumptions.Jens Schweikhardt
The Compliant Solution as of JUN-14-2010 is a mess:
It uses
copy
andpath
when it should use only one of them in addition totemp
.path
is undeclared and the test forif (copy == NULL)
is always true. I find it rather embarrasing that bugs like this slip in such a carefully crafted coding guideline. Has it never been considered to at least test compile the Compliant Solutions (at least with-c
; nomain()
required, but correct headers, please)? I don't dare asking for test linting... :-)David Svoboda
I've fixed the compliant solution (and even compiled it
Robert Seacord (Manager)
This recommendation and example appears to be entirely redundant with the getenv() example in STR31-C. We should probably eliminate this, because recommendation instead of the example, because this code example violates STR31-C.
Joseph C. Sible
A much simpler compliant solution is possible with POSIX, by using
strdup
.