Project

General

Profile

Bug #344

su fails to check malloc's return value

Added by Albert Lee over 10 years ago. Updated almost 9 years ago.

Status:
Resolved
Priority:
Low
Assignee:
Category:
cmd - userland programs
Start date:
2010-10-13
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

For reference, this was reported as CVE-2010-3503.

#1

Updated by Albert Lee over 10 years ago

  • Priority changed from Normal to Low

Reassess priority, this bug does not affect security.

#2

Updated by Albert Lee over 10 years ago

  • Subject changed from Fix CVE-2010-3503 to su fails to check malloc's return value
#3

Updated by Rob De Langhe over 10 years ago

uploaded a webrev with the change:

http://cr.illumos.org/view/a8i2l5mn/

up to you to validate (and accept?) it...

#4

Updated by Damian Wojslaw over 10 years ago

Hi
You need not to work on usr/src/tools/env/illumos.sh. Please, make a private of this file and modify it.

#5

Updated by Rob De Langhe over 10 years ago

  • Status changed from New to Feedback

thx to some valuable comments from co-supporter of Illumos, I tweaked my (new) dev environment, and uploaded a new webrev for this bug :

http://cr.illumos.org/view/8zdueyui

-> pls comment, and/or acknowledge this (minor) fix

#6

Updated by Damian Wojslaw over 10 years ago

Looks okay to me, now.
As I understand, you've built and installed it?

#7

Updated by Rob De Langhe over 10 years ago

yes, have build and installed it (of course extremely minor change, so no impact and hard to simulate the issue)

Output from "hg pbchk" :

$ cat /tmp/hg-pbchk.out
Copyright check:

C style check:
Could not execute cstyle: [Errno 2] No such file or directory

Header format check:

Java style check:

Mapfile comment check:

File permission check:

Keywords check:

Comments check:

Checking for new tags:

Checking for multiple heads (or branches):

Checking for branch changes:

Checking for uncommitted changes:
Warning: the following files have uncommitted changes:
usr/src/cmd/su/su.c

Checking for merges:

#8

Updated by Albert Lee over 10 years ago

Rob, I tried to contact you on IRC, but I guess you missed my message.

I already have this done:
http://pkgdev.openindiana.org/~trisk/old/il_cve-2010-3503/

...which is identical to your changes except for style guidelines.

If you want to pursue getting this put back, I can review it (and approve it once you get another reviewer). There is a testcase for the issue attached to the CVE iirc, which I used.

#9

Updated by Rob De Langhe over 10 years ago

Albert,

yes please, review it (and approve, hopefully). If you can help me to identify other reviewers, that would be nice.

rgds
Rob

#10

Updated by Rob De Langhe over 10 years ago

all,

please review my change:

(it has preliminary been reviewed already by Damian Wojslaw)

% hg pbchkk
Copyright check:

C style check:

Header format check:

Java style check:

Mapfile comment check:

File permission check:

Keywords check:

Comments check:
These IDs appear more than once in your comments:
  344

Checking for new tags:

Checking for multiple heads (or branches):

Checking for branch changes:

Checking for uncommitted changes:

Checking for merges:

% hg -v outgoing

running ssh anonhg@hg.illumos.org "hg -R illumos-gate serve --stdio" 
comparing with ssh://anonhg@hg.illumos.org/illumos-gate
searching for changes

changeset:   13246:c39379982564
user:        rob <rob@openindiana.local>
date:        Mon Dec 13 13:31:16 2010 +0100

description:
    344 su fails to check malloc's return value

modified:
   usr/src/cmd/su/su.c

changeset:   13247:22dfc1060a79
tag:         tip
user:        rob <rob@openindiana.local>
date:        Tue Dec 14 13:20:18 2010 +0100

description:
    344 su fails to check malloc's return value

modified:
   usr/src/cmd/su/su.c

% hg export tip
# HG changeset patch
# User rob <rob@openindiana.local>
# Date 1292329218 -3600
# Node ID 22dfc1060a79ef2d710209f97581ff377fa07d2f
# Parent  c393799825649271d04aab7eb8363164d626e45f
344 su fails to check malloc's return value

diff -r c39379982564 -r 22dfc1060a79 usr/src/cmd/su/su.c
--- a/usr/src/cmd/su/su.c    Mon Dec 13 13:31:16 2010 +0100
+++ b/usr/src/cmd/su/su.c    Tue Dec 14 13:20:18 2010 +0100
@@ -535,9 +535,10 @@
                         malloc(strlen(initenv[j])
                         + strlen(initvar)
                         + 2);
-                                        if (var == NULL) {
-                       perror("malloc"); exit(4);
-                                        }
+                    if (var == NULL) {
+                        perror("malloc");
+                        exit(4);
+                    }
                     (void) strcpy(var, initenv[j]);
                     (void) strcat(var, "=");
                     (void) strcat(var, initvar);

#11

Updated by Albert Lee over 10 years ago

The diff is incorrect.

#12

Updated by Albert Lee over 10 years ago

Can you update the patch to be against the correct tree? (It would likely be identical to the one I showed you).

#13

Updated by Garrett D'Amore almost 9 years ago

  • Status changed from Feedback to Resolved
  • Assignee changed from Albert Lee to Milan Jurik
  • % Done changed from 0 to 100
  • Difficulty set to Medium

Resolved in:

garrett@velocity{11}> hg head
changeset: 13695:2d025fa9b1e1
tag: tip
user: Milan Jurik <>
date: Tue May 15 12:48:44 2012 -0700
description:
344 su fails to check malloc's return value
Reviewed by: Albert Lee <>
Reviewed by: Enrico Papi <>
Reviewed by: Garrett D'Amore <>
Approved by: Garrett D'Amore <>

Also available in: Atom PDF