openssh bugs - Oct 2013 - [Bug 2163] New: unchecked returned value from pam_get_item()

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

            Bug ID: 2163
           Summary: unchecked returned value from pam_get_item()
           Product: Portable OpenSSH
           Version: -current
          Hardware: All
                OS: All
            Status: NEW
          Severity: minor
          Priority: P5
         Component: PAM support
          Assignee: unassigned-bugs at mindrot.org
          Reporter: loganaden at gmail.com

Created attachment 2356
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2356&action=edit
auth-pam.c.diff

in auth-pam.c,

A call is made to pam_get_item() but the returned value isn't checked.

In other places, the returned value to pam_get_item() has been checked.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

--- Comment #1 from Loganaden Velvindron <loganaden at gmail.com> ---
Created attachment 2357
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2357&action=edit
sftp.c.diff

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

--- Comment #2 from Loganaden Velvindron <loganaden at gmail.com> ---
Created attachment 2358
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2358&action=edit
session.c.diff

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

--- Comment #3 from Loganaden Velvindron <loganaden at gmail.com> ---
Created attachment 2359
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2359&action=edit
gss-serv.c.diff

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

--- Comment #4 from Loganaden Velvindron <loganaden at gmail.com> ---
Created attachment 2360
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2360&action=edit
ssh-keyscan.c.diff

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

--- Comment #5 from Loganaden Velvindron <loganaden at gmail.com> ---
Created attachment 2361
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2361&action=edit
ssh-pkcs11-client.c.diff

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

--- Comment #6 from Loganaden Velvindron <loganaden at gmail.com> ---
Created attachment 2362
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2362&action=edit
sshconnect.c.diff

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

Loganaden Velvindron <loganaden at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2362|0                           |1
           is patch|                            |

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

--- Comment #7 from Loganaden Velvindron <loganaden at gmail.com> ---
Created attachment 2363
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2363&action=edit
sshconnect2.c.diff

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

--- Comment #8 from Loganaden Velvindron <loganaden at gmail.com> ---
Similar issues in other files.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dtucker at zip.com.au
             Blocks|                            |2130

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |djm at mindrot.org

--- Comment #9 from Damien Miller <djm at mindrot.org> ---
Comment on attachment 2357
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2357
sftp.c.diff

IMO it's better return return a NULL array on failure. I'll make a
patch.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2357|0                           |1
        is obsolete|                            |

--- Comment #10 from Damien Miller <djm at mindrot.org> ---
Created attachment 2381
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2381&action=edit
sftp.c leak fix

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

--- Comment #11 from Loganaden Velvindron <loganaden at gmail.com> ---
(In reply to Damien Miller from comment #10)
> Created attachment 2381 [details]
> sftp.c leak fix
Looks better !

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2356|0                           |1
        is obsolete|                            |

--- Comment #12 from Darren Tucker <dtucker at zip.com.au> ---
Comment on attachment 2356
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2356
auth-pam.c.diff

auth-pam.c change applied, thanks.  still yet to look at the others.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

--- Comment #13 from Darren Tucker <dtucker at zip.com.au> ---
Comment on attachment 2358
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2358
session.c.diff
>-	int n_bytes;
>+	int n_bytes = 0;
What's the intent here, silencing a compiler warning?   n_bytes always
gets initialized before use, in the case of protocol 1 in
session_pty_req():

        /* for SSH1 the tty modes length is not given */
        if (!compat20)
                n_bytes = packet_remaining();
        tty_parse_modes(s->ttyfd, &n_bytes);

and in the protocol 2 case at the start of tty_parse_modes:


        if (compat20) {
                *n_bytes_ptr = packet_get_int();
                if (*n_bytes_ptr == 0)
                        return;

I can imagine a compiler not figuring this out, though.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2360|                            |ok+
              Flags|                            |

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2361|                            |ok+
              Flags|                            |

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2362|                            |ok+
              Flags|                            |

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

--- Comment #14 from Loganaden Velvindron <loganaden at gmail.com> ---
(In reply to Darren Tucker from comment #13)
> Comment on attachment 2358 [details]
> session.c.diff
> 
> >-	int n_bytes;
> >+	int n_bytes = 0;
> 
> What's the intent here, silencing a compiler warning?   n_bytes
> always gets initialized before use, in the case of protocol 1 in
> session_pty_req():
> 
>         /* for SSH1 the tty modes length is not given */
>         if (!compat20)
>                 n_bytes = packet_remaining();
>         tty_parse_modes(s->ttyfd, &n_bytes);
> 
> and in the protocol 2 case at the start of tty_parse_modes:
> 
> 
>         if (compat20) {
>                 *n_bytes_ptr = packet_get_int();
>                 if (*n_bytes_ptr == 0)
>                         return;
> 
> I can imagine a compiler not figuring this out, though.
Yep :-) The compiler didn't figure it out. I should have looked in
other parts of the code.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2188

--- Comment #15 from Damien Miller <djm at mindrot.org> ---
Retarget incomplete bugs / feature requests to 6.6 release

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|2130                        |

--- Comment #16 from Damien Miller <djm at mindrot.org> ---
Retarget incomplete bugs / feature requests to 6.6 release

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2226

--- Comment #17 from Damien Miller <djm at mindrot.org> ---
Retarget to 6.7 release, since 6.6 was mostly bugfixing.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|2188                        |

--- Comment #18 from Damien Miller <djm at mindrot.org> ---
Remove from 6.6 tracking bug

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2266

--- Comment #19 from Damien Miller <djm at mindrot.org> ---
Retarget incomplete bugs to 6.8 release.

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|2226                        |

--- Comment #20 from Damien Miller <djm at mindrot.org> ---
These bugs are no longer targeted at the imminent 6.7 release

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|2266                        |

--- Comment #21 from Damien Miller <djm at mindrot.org> ---
OpenSSH 6.8 is approaching release and closed for major work. Retarget
these bugs for the next release.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2360

--- Comment #22 from Damien Miller <djm at mindrot.org> ---
Retarget to 6.9

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2358|                            |ok+
              Flags|                            |

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2163

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|2360                        |

--- Comment #23 from Damien Miller <djm at mindrot.org> ---
detarget for now, will deal with after 6.9 release

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.