Bug 66386
| Summary: | Buffer overrun in the 'in_pcbopts' function. | ||
|---|---|---|---|
| Product: | Base System | Reporter: | Andrei Iltchenko <iltchenko> |
| Component: | kern | Assignee: | Maxim Konovalov <maxim> |
| Status: | Closed FIXED | ||
| Severity: | Affects Only Me | ||
| Priority: | Normal | ||
| Version: | Unspecified | ||
| Hardware: | Any | ||
| OS: | Any | ||
On Sat, 8 May 2004, 06:33-0700, Andrei Iltchenko wrote: [...] > >Description: > The 'ip_pcbopts' function from 'ip_output.c' features a buffer overrun which > takes place whenever either an 'IPOPT_LSRR' or an 'IPOPT_SSRR' option is supplied. > Here's the offending piece of code: > /* > * Then copy rest of options back > * to close up the deleted entry. > */ > ovbcopy((caddr_t)(&cp[IPOPT_OFFSET+1] + > sizeof(struct in_addr)), > (caddr_t)&cp[IPOPT_OFFSET+1], > (unsigned)cnt + sizeof(struct in_addr)); > break; > > The problem in question is the last argument in the above call to > 'ovbcopy', which runs over the end of the buffer by 7 bytes (i386). > >How-To-Repeat: > > >Fix: > The call to 'ovbcopy' should be rewritten to read: > /* > * Then copy rest of options back > * to close up the deleted entry. > */ > ovbcopy((caddr_t)(&cp[IPOPT_OFFSET+1] + > sizeof(struct in_addr)), > (caddr_t)&cp[IPOPT_OFFSET+1], > (unsigned)cnt - IPOPT_MINOFF-1); > break; Did you mean "(unsigned)cnt - (IPOPT_MINOFF - 1))"? Index: ip_output.c =================================================================== RCS file: /home/ncvs/src/sys/netinet/ip_output.c,v retrieving revision 1.215 diff -u -r1.215 ip_output.c --- ip_output.c 14 Apr 2004 01:13:14 -0000 1.215 +++ ip_output.c 9 May 2004 13:40:41 -0000 @@ -1735,7 +1735,7 @@ */ bcopy((&cp[IPOPT_OFFSET+1] + sizeof(struct in_addr)), &cp[IPOPT_OFFSET+1], - (unsigned)cnt + sizeof(struct in_addr)); + (unsigned)cnt - (IPOPT_MINOFF - 1)); break; } } %%% -- Maxim Konovalov Yes, I did mean "(unsigned)cnt - (IPOPT_MINOFF - 1))". Sorry for the slipup. Regards, Andrei. --- Maxim Konovalov <maxim@macomnet.ru> wrote: > > Did you mean "(unsigned)cnt - (IPOPT_MINOFF - 1))"? > > Index: ip_output.c > =================================================================== > RCS file: /home/ncvs/src/sys/netinet/ip_output.c,v > retrieving revision 1.215 > diff -u -r1.215 ip_output.c > --- ip_output.c 14 Apr 2004 01:13:14 -0000 1.215 > +++ ip_output.c 9 May 2004 13:40:41 -0000 > @@ -1735,7 +1735,7 @@ > */ > bcopy((&cp[IPOPT_OFFSET+1] + sizeof(struct > in_addr)), > &cp[IPOPT_OFFSET+1], > - (unsigned)cnt + sizeof(struct in_addr)); > + (unsigned)cnt - (IPOPT_MINOFF - 1)); > break; > } > } > %%% > > -- > Maxim Konovalov __________________________________ Do you Yahoo!? Win a $20,000 Career Makeover at Yahoo! HotJobs http://hotjobs.sweepstakes.yahoo.com/careermakeover Responsible Changed From-To: freebsd-bugs->maxim Over to IP options addict. State Changed From-To: open->patched Fixed in -CURRENT, thanks! State Changed From-To: patched->closed Fixed in -STABLE as well. |
The 'ip_pcbopts' function from 'ip_output.c' features a buffer overrun which takes place whenever either an 'IPOPT_LSRR' or an 'IPOPT_SSRR' option is supplied. Here's the offending piece of code: /* * Then copy rest of options back * to close up the deleted entry. */ ovbcopy((caddr_t)(&cp[IPOPT_OFFSET+1] + sizeof(struct in_addr)), (caddr_t)&cp[IPOPT_OFFSET+1], (unsigned)cnt + sizeof(struct in_addr)); break; The problem in question is the last argument in the above call to 'ovbcopy', which runs over the end of the buffer by 7 bytes (i386). Fix: The call to 'ovbcopy' should be rewritten to read: /* * Then copy rest of options back * to close up the deleted entry. */ ovbcopy((caddr_t)(&cp[IPOPT_OFFSET+1] + sizeof(struct in_addr)), (caddr_t)&cp[IPOPT_OFFSET+1], (unsigned)cnt - IPOPT_MINOFF-1); break;