Project

General

Profile

Bug #344

su fails to check malloc's return value

Added by Albert Lee about 9 years ago. Updated over 7 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:

Description

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

History

#1

Updated by Albert Lee about 9 years ago

  • Priority changed from Normal to Low

Reassess priority, this bug does not affect security.

#2

Updated by Albert Lee about 9 years ago

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

Updated by Rob De Langhe almost 9 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 almost 9 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 almost 9 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 almost 9 years ago

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

#7

Updated by Rob De Langhe almost 9 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 almost 9 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 almost 9 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 almost 9 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 almost 9 years ago

The diff is incorrect.

#12

Updated by Albert Lee almost 9 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 over 7 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