login: add a check for a post-condition when exporting the environment
ClosedPublic
Actions

Authored by pfg on Oct 27 2020, 4:29 AM.

Details

Reviewers

Commits

rS368776: login(1): when exporting variables check the result of setenv(3)

Summary

FreeBSD's login code is used as an example case in the "Principles of Secure
Coding" coursera course. While FreeBSD correctly checks for wrong
preconditions it could also check for the setenv post-conditon.

It is clear that setenv(3) will not fail, however it wouldn't hurt to make sure
the return value matches our expectacions.

Credit for this change is due to Matthew Bishop, PhD

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

pfg created this revision.Oct 27 2020, 4:29 AM

Herald added a subscriber: imp. · View Herald TranscriptOct 27 2020, 4:30 AM

pfg requested review of this revision.Oct 27 2020, 4:30 AM

Harbormaster completed remote builds in B34407: Diff 78795.Oct 27 2020, 4:30 AM

pfg retitled this revision from login: add a check for a precondition when exporting the environment to login: add a check for a post-condition when exporting the environment.Dec 19 2020, 2:01 AM

pfg edited the summary of this revision. (Show Details)Dec 19 2020, 2:14 AM

This revision was not accepted when it landed; it landed in state Needs Review.Dec 19 2020, 2:24 AM

Closed by commit rS368776: login(1): when exporting variables check the result of setenv(3) (authored by pfg). · Explain Why

This revision was automatically updated to reflect the committed changes.

pfg added a commit: rS368776: login(1): when exporting variables check the result of setenv(3).

Looks like export is only ever used in export_pam_environment(), where its return value is ignored.

In D26966#618725, @yuripv wrote:

Looks like export is only ever used in export_pam_environment(), where its return value is ignored.

Yes, I see :-/
So the change is useless, as are the checks for the pre-conditions.

It was really nice to see FreeBSD mentioned in a software security course though.

head/usr.bin/login/login.c
810	This is actually -1 : 1 is vener returned by setenv(3).

In D26966#618727, @pfg wrote:

In D26966#618725, @yuripv wrote:

Looks like export is only ever used in export_pam_environment(), where its return value is ignored.

Yes, I see :-/
So the change is useless, as are the checks for the pre-conditions.

It was really nice to see FreeBSD mentioned in a software security course though.

Preconditions are actually useful in that we do NOT set LD_* or other forbidden variables.

Revision Contents
Changeset List

Path

Size

head/

usr.bin/

5 lines

Diff 80924

View Options

head/usr.bin/login/login.c

login: add a check for a post-condition when exporting the environmentClosedPublicActions