Bug 1464409

Summary: shp.fdstatus may be accessed with -1 offset, corrupting last element of shp.fdptrs
Product: [Fedora] Fedora Reporter: Siteshwar Vashisht <svashisht>
Component: kshAssignee: Siteshwar Vashisht <svashisht>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: unspecified    
Version: 27CC: kdudka, mhlavink, pandrade, qe-baseos-apps, svashisht
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: ksh-20120801-42.fc28 Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: 1463312 Environment:
Last Closed: 2017-08-29 16:43:12 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 1463312    
Bug Blocks:    
Attachments:
Description Flags
Fix memory corruption caused by accessing array with negative index
kdudka: review-
Fix memory corruption caused by accessing array with negative index kdudka: review+

Comment 1 Jan Kurik 2017-08-15 08:57:49 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 27 development cycle.
Changing version to '27'.

Comment 2 Siteshwar Vashisht 2017-08-28 13:18:34 UTC
Created attachment 1319079 [details]
Fix memory corruption caused by accessing array with negative index

Comment 3 Kamil Dudka 2017-08-28 15:48:24 UTC
Comment on attachment 1319079 [details]
Fix memory corruption caused by accessing array with negative index

> diff --git a/src/cmd/ksh93/sh/io.c b/src/cmd/ksh93/sh/io.c
> --- a/src/cmd/ksh93/sh/io.c
> +++ b/src/cmd/ksh93/sh/io.c
> @@ -403,38 +403,50 @@ static short		filemapsize;
>  
>  /* ======== input output and file copying ======== */
>  
> -int  sh_iovalidfd(Shell_t *shp, int fd)
> +bool  sh_iovalidfd(Shell_t *shp, int fd)
>  {
>  	Sfio_t		**sftable = shp->sftable;
>  	int		max,n, **fdptrs = shp->fdptrs;
> -	unsigned char	*fdstatus = shp->fdstatus;
> +	unsigned int	*fdstatus = shp->fdstatus;

This looks incorrect to me.  shp->fdstatus is defined as (unsigned char *)
in <ksh93/include/defs.h>.  We should not cast it to an incompatible pointer
type just in this function because it would result in undefined behavior.

Comment 4 Siteshwar Vashisht 2017-08-28 16:09:04 UTC
Created attachment 1319143 [details]
Fix memory corruption caused by accessing array with negative index

Comment 5 Kamil Dudka 2017-08-28 16:17:10 UTC
Comment on attachment 1319143 [details]
Fix memory corruption caused by accessing array with negative index

Looks good.