Re: [CFR] diskpart(1) buffer overflow fix
Le 2002-12-02, Peter Pentchev écrivait : > Ahhh; of course this would be better. Updated patch attached. Looks fine. Thomas. -- [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-hackers" in the body of the message
Re: [CFR] diskpart(1) buffer overflow fix
On Mon, Dec 02, 2002 at 01:37:52PM +0100, Thomas Quinot wrote:
> Le 2002-12-02, Peter Pentchev ?crivait :
>
> > > Attached are two patches: a trivial one which just fixes up two problems
> > > in diskpart's argument parsing, and a more complex one, which does it
> > > "the right way" IMHO, using getopt(3).
>
> The getopt-based version sounds better to me.
>
> > + case 'd':
> > + dflag++;
> > + if (pflag)
> > + usage();
> > + break;
> > +
> > + case 'p':
> > + if (dflag)
> > + usage();
> > + pflag++;
> > + break;
>
> I'd remove both tests and replace them with a single
> if (pflag && dflag) usage()
> after all arguments have been processed.
Ahhh; of course this would be better. Updated patch attached.
G'luck,
Peter
--
Peter Pentchev [EMAIL PROTECTED][EMAIL PROTECTED]
PGP key:http://people.FreeBSD.org/~roam/roam.key.asc
Key fingerprint FDBA FD79 C26F 3C51 C95E DF9E ED18 B68D 1619 4553
If there were no counterfactuals, this sentence would not have been paradoxical.
Index: src/usr.sbin/diskpart/diskpart.c
===
RCS file: /home/ncvs/src/usr.sbin/diskpart/Attic/diskpart.c,v
retrieving revision 1.11.2.1
diff -u -r1.11.2.1 diskpart.c
--- src/usr.sbin/diskpart/diskpart.c7 Jan 2002 06:00:23 - 1.11.2.1
+++ src/usr.sbin/diskpart/diskpart.c2 Dec 2002 12:45:27 -
@@ -55,6 +55,7 @@
#include
#include
#include
+#include
#definefor_now /* show all of `c' partition for disklabel */
#defineNPARTITIONS 8
@@ -126,22 +127,29 @@
int threshhold, numcyls[NPARTITIONS], startcyl[NPARTITIONS];
int totsize = 0;
char *lp, *tyname;
+ int ch;
- argc--, argv++;
+ while ((ch = getopt(argc, argv, "dps:")) != EOF)
+ switch (ch) {
+ case 'd':
+ dflag++;
+ break;
+
+ case 'p':
+ pflag++;
+ break;
+
+ case 's':
+ totsize = atoi(optarg);
+ break;
+ }
+ argc -= optind;
+ argv += optind;
+
+ if (dflag && pflag)
+ usage();
if (argc < 1)
usage();
- if (argc > 0 && strcmp(*argv, "-p") == 0) {
- pflag++;
- argc--, argv++;
- }
- if (argc > 0 && strcmp(*argv, "-d") == 0) {
- dflag++;
- argc--, argv++;
- }
- if (argc > 1 && strcmp(*argv, "-s") == 0) {
- totsize = atoi(argv[1]);
- argc += 2, argv += 2;
- }
dp = getdiskbyname(*argv);
if (dp == NULL) {
if (isatty(0))
msg38429/pgp0.pgp
Description: PGP signature
Re: [CFR] diskpart(1) buffer overflow fix
Le 2002-12-02, Peter Pentchev écrivait : > > Attached are two patches: a trivial one which just fixes up two problems > > in diskpart's argument parsing, and a more complex one, which does it > > "the right way" IMHO, using getopt(3). The getopt-based version sounds better to me. > + case 'd': > + dflag++; > + if (pflag) > + usage(); > + break; > + > + case 'p': > + if (dflag) > + usage(); > + pflag++; > + break; I'd remove both tests and replace them with a single if (pflag && dflag) usage() after all arguments have been processed. Thomas. -- [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-hackers" in the body of the message
Re: [CFR] diskpart(1) buffer overflow fix
On Mon, Dec 02, 2002 at 01:58:09PM +0200, Peter Pentchev wrote:
> Hi,
>
> As noted on the vuln-dev list recently, the diskpart(1) program in
> -stable is susceptible to a buffer overflow in the parsing of
> command-line arguments. This is a low-risk problem, since diskpart(1)
> is not - and has never been, and has no reason to ever be - a privileged
> program, but still, there should be no harm in fixing it :)
>
> Attached are two patches: a trivial one which just fixes up two problems
> in diskpart's argument parsing, and a more complex one, which does it
> "the right way" IMHO, using getopt(3).
>
> Comments?
And a comment from myself: of course it would have been way better if I
had actually attached the patches...
G'luck,
Peter
--
Peter Pentchev [EMAIL PROTECTED][EMAIL PROTECTED]
PGP key:http://people.FreeBSD.org/~roam/roam.key.asc
Key fingerprint FDBA FD79 C26F 3C51 C95E DF9E ED18 B68D 1619 4553
If I were you, who would be reading this sentence?
Index: src/usr.sbin/diskpart/diskpart.c
===
RCS file: /home/ncvs/src/usr.sbin/diskpart/Attic/diskpart.c,v
retrieving revision 1.11.2.1
diff -u -r1.11.2.1 diskpart.c
--- src/usr.sbin/diskpart/diskpart.c7 Jan 2002 06:00:23 - 1.11.2.1
+++ src/usr.sbin/diskpart/diskpart.c2 Dec 2002 11:32:58 -
@@ -128,8 +128,6 @@
char *lp, *tyname;
argc--, argv++;
- if (argc < 1)
- usage();
if (argc > 0 && strcmp(*argv, "-p") == 0) {
pflag++;
argc--, argv++;
@@ -140,8 +138,10 @@
}
if (argc > 1 && strcmp(*argv, "-s") == 0) {
totsize = atoi(argv[1]);
- argc += 2, argv += 2;
+ argc -= 2, argv += 2;
}
+ if (argc < 1)
+ usage();
dp = getdiskbyname(*argv);
if (dp == NULL) {
if (isatty(0))
Index: src/usr.sbin/diskpart/diskpart.c
===
RCS file: /home/ncvs/src/usr.sbin/diskpart/Attic/diskpart.c,v
retrieving revision 1.11.2.1
diff -u -r1.11.2.1 diskpart.c
--- src/usr.sbin/diskpart/diskpart.c7 Jan 2002 06:00:23 - 1.11.2.1
+++ src/usr.sbin/diskpart/diskpart.c20 Nov 2002 15:14:46 -
@@ -55,6 +55,7 @@
#include
#include
#include
+#include
#definefor_now /* show all of `c' partition for disklabel */
#defineNPARTITIONS 8
@@ -126,22 +127,30 @@
int threshhold, numcyls[NPARTITIONS], startcyl[NPARTITIONS];
int totsize = 0;
char *lp, *tyname;
+ int ch;
- argc--, argv++;
+ while ((ch = getopt(argc, argv, "dps:")) != EOF)
+ switch (ch) {
+ case 'd':
+ dflag++;
+ if (pflag)
+ usage();
+ break;
+
+ case 'p':
+ if (dflag)
+ usage();
+ pflag++;
+ break;
+
+ case 's':
+ totsize = atoi(optarg);
+ break;
+ }
+ argc -= optind;
+ argv += optind;
if (argc < 1)
usage();
- if (argc > 0 && strcmp(*argv, "-p") == 0) {
- pflag++;
- argc--, argv++;
- }
- if (argc > 0 && strcmp(*argv, "-d") == 0) {
- dflag++;
- argc--, argv++;
- }
- if (argc > 1 && strcmp(*argv, "-s") == 0) {
- totsize = atoi(argv[1]);
- argc += 2, argv += 2;
- }
dp = getdiskbyname(*argv);
if (dp == NULL) {
if (isatty(0))
msg38427/pgp0.pgp
Description: PGP signature
[CFR] diskpart(1) buffer overflow fix
Hi, As noted on the vuln-dev list recently, the diskpart(1) program in -stable is susceptible to a buffer overflow in the parsing of command-line arguments. This is a low-risk problem, since diskpart(1) is not - and has never been, and has no reason to ever be - a privileged program, but still, there should be no harm in fixing it :) Attached are two patches: a trivial one which just fixes up two problems in diskpart's argument parsing, and a more complex one, which does it "the right way" IMHO, using getopt(3). Comments? G'luck, Peter -- Peter Pentchev [EMAIL PROTECTED][EMAIL PROTECTED] PGP key:http://people.FreeBSD.org/~roam/roam.key.asc Key fingerprint FDBA FD79 C26F 3C51 C95E DF9E ED18 B68D 1619 4553 .siht ekil ti gnidaer eb d'uoy ,werbeH ni erew ecnetnes siht fI msg38425/pgp0.pgp Description: PGP signature
