• Address concerns of Chris Palmer from the Android security team
– possible integer overflows in memory allocation, mostly
    ‣ multiplication: all are checked now
    ‣ addition: reviewed them, most were “proven” or guessed to be
      “almost” impossible to run over (e.g. when we have a string
      whose length is taken it is assumed that the length will be
      more than only a few bytes below SIZE_MAX, since code and
      stack have to fit); some are checked now (e.g. when one of
      the summands is an off_t); most of the unchecked ones are
      annotated now
    ⇒ cost (MirBSD/i386 static): +76 .text
    ⇒ cost (Debian sid/i386): +779 .text  -4 .data
  – on Linux targets, setuid() setresuid() setresgid() can fail
    with EAGAIN; check for that and, if so, warn once and retry
    infinitely (other targets to be added later once we know that
    they are “insane”)
    ⇒ cost (Debian sid/i386): +192 .text (includes .rodata)
• setmode.c: Do overflow checking for realloc() too; switch back
  from calloc() to a checked malloc() for simplification while there
• define -DIN_MKSH and let setmode.c look a tad nicer while here
			
			
This commit is contained in:
		
							
								
								
									
										14
									
								
								var.c
									
									
									
									
									
								
							
							
						
						
									
										14
									
								
								var.c
									
									
									
									
									
								
							@@ -26,7 +26,7 @@
 | 
			
		||||
#include <sys/sysctl.h>
 | 
			
		||||
#endif
 | 
			
		||||
 | 
			
		||||
__RCSID("$MirOS: src/bin/mksh/var.c,v 1.112 2010/08/28 20:22:24 tg Exp $");
 | 
			
		||||
__RCSID("$MirOS: src/bin/mksh/var.c,v 1.113 2010/09/14 21:26:19 tg Exp $");
 | 
			
		||||
 | 
			
		||||
/*
 | 
			
		||||
 * Variables
 | 
			
		||||
@@ -649,10 +649,13 @@ exportprep(struct tbl *vp, const char *val)
 | 
			
		||||
{
 | 
			
		||||
	char *xp;
 | 
			
		||||
	char *op = (vp->flag&ALLOC) ? vp->val.s : NULL;
 | 
			
		||||
	int namelen = strlen(vp->name);
 | 
			
		||||
	int vallen = strlen(val) + 1;
 | 
			
		||||
	size_t namelen, vallen;
 | 
			
		||||
 | 
			
		||||
	namelen = strlen(vp->name);
 | 
			
		||||
	vallen = strlen(val) + 1;
 | 
			
		||||
 | 
			
		||||
	vp->flag |= ALLOC;
 | 
			
		||||
	/* since name+val are both in memory this can go unchecked */
 | 
			
		||||
	xp = alloc(namelen + 1 + vallen, vp->areap);
 | 
			
		||||
	memcpy(vp->val.s = xp, vp->name, namelen);
 | 
			
		||||
	xp += namelen;
 | 
			
		||||
@@ -1317,9 +1320,10 @@ arraysearch(struct tbl *vp, uint32_t val)
 | 
			
		||||
		news = curr;
 | 
			
		||||
	} else
 | 
			
		||||
		news = NULL;
 | 
			
		||||
	len = strlen(vp->name) + 1;
 | 
			
		||||
	if (!news) {
 | 
			
		||||
		news = alloc(offsetof(struct tbl, name[0]) + len, vp->areap);
 | 
			
		||||
		len = strlen(vp->name);
 | 
			
		||||
		checkoktoadd(len, 1 + offsetof(struct tbl, name[0]));
 | 
			
		||||
		news = alloc(offsetof(struct tbl, name[0]) + ++len, vp->areap);
 | 
			
		||||
		memcpy(news->name, vp->name, len);
 | 
			
		||||
	}
 | 
			
		||||
	news->flag = (vp->flag & ~(ALLOC|DEFINED|ISSET|SPECIAL)) | AINDEX;
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user