Page MenuHomeFreeBSD
Differential D26966

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

Authored by pfg on Oct 27 2020, 4:29 AM.
Tags
None
Referenced Files
F106106571: D26966.diff
Wed, Dec 25, 12:25 PM
Unknown Object (File)
Thu, Dec 5, 4:41 AM
Unknown Object (File)
Mon, Dec 2, 9:15 AM
Unknown Object (File)
Mon, Dec 2, 9:15 AM
Unknown Object (File)
Mon, Dec 2, 9:15 AM
Unknown Object (File)
Mon, Dec 2, 8:49 AM
Unknown Object (File)
Nov 7 2024, 8:43 AM
Unknown Object (File)
Oct 14 2024, 3:40 AM
View All 56 Files
Subscribers
imp
yuripv

Details

Reviewers
des
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).
yuripv added a subscriber: yuripv.Dec 19 2020, 3:33 AM

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

pfg added a comment.Dec 19 2020, 3:47 AM
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).

yuripv added a comment.Dec 19 2020, 4:00 AM
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

PathSize
head/
usr.bin/
login/
5 lines

Diff 80924

View Options

head/usr.bin/login/login.c

Loading...