Closed
Bug 173847
Opened 22 years ago
Closed 18 years ago
flawfinder warnings in gfx
Categories
(Core :: Printing: Output, defect, P1)
Tracking
()
RESOLVED
WORKSFORME
mozilla1.3beta
People
(Reporter: morse, Assigned: rods)
References
Details
Attachments
(1 file)
|
81.85 KB,
text/html
|
Details |
Heikki ran flawfinder (http://www.dwheeler.com/flawfinder) on Mozilla 1.0.1 branch. flawfinder found 106 warnings in gfx code (3390-3495). Go through that list and for each warning: * If it is false positive, comment here why it is not an issue * If it is a real issue, make patch for it here and let's get them checked in In addition to checking the branch, also check the trunk. 3390) gfx/src/nsNameValuePairDB.cpp:53 [4] (format) printf: if format strings can be influenced by an attacker, they can be exploited. Use a constant for the format specification. 3391) gfx/src/nsPrintOptionsImpl.cpp:1109 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3392) gfx/src/beos/nsDeviceContextSpecB.cpp:295 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3393) gfx/src/beos/nsDeviceContextSpecB.cpp:296 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3394) gfx/src/beos/nsDeviceContextSpecB.cpp:299 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3395) gfx/src/beos/nsDeviceContextSpecB.cpp:310 [2] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. Risk is low because the source is a constant string. 3396) gfx/src/beos/nsDeviceContextSpecB.cpp:312 [2] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. Risk is low because the source is a constant string. 3397) gfx/src/beos/nsDeviceContextSpecB.cpp:332 [2] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. Risk is low because the source is a constant string. 3398) gfx/src/beos/nsDeviceContextSpecB.cpp:335 [4] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. 3399) gfx/src/beos/nsFontMetricsBeOS.cpp:173 [4] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. 3400) gfx/src/gtk/nsDeviceContextSpecG.cpp:408 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3401) gfx/src/gtk/nsDeviceContextSpecG.cpp:410 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3402) gfx/src/gtk/nsDeviceContextSpecG.cpp:412 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3403) gfx/src/gtk/nsDeviceContextSpecG.cpp:414 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3404) gfx/src/gtk/nsFontMetricsGTK.cpp:3745 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3405) gfx/src/gtk/nsFontMetricsGTK.cpp:3872 [4] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. 3406) gfx/src/gtk/nsFontMetricsGTK.h:84 [4] (format) printf: if format strings can be influenced by an attacker, they can be exploited. Use a constant for the format specification. 3407) gfx/src/gtk/nsFontMetricsGTK.h:112 [4] (format) printf: if format strings can be influenced by an attacker, they can be exploited. Use a constant for the format specification. 3408) gfx/src/gtk/nsPrintdGTK.c:94 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3409) gfx/src/gtk/nsPrintdGTK.c:96 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3410) gfx/src/gtk/nsPrintdGTK.c:142 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3411) gfx/src/mac/nsCoreGraphicsGlue.cpp:99 [4] (format) printf: if format strings can be influenced by an attacker, they can be exploited. Use a constant for the format specification. 3412) gfx/src/mac/nsUnicodeMappingUtil.cpp:335 [4] (format) snprintf: if format strings can be influenced by an attacker, they can be exploited. Use a constant for the format specification. 3413) gfx/src/mac/nsUnicodeMappingUtil.cpp:397 [4] (format) snprintf: if format strings can be influenced by an attacker, they can be exploited. Use a constant for the format specification. 3414) gfx/src/os2/nsDeviceContextSpecOS2.cpp:374 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3415) gfx/src/os2/nsDeviceContextSpecOS2.cpp:377 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3416) gfx/src/os2/nsDeviceContextSpecOS2.cpp:662 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3417) gfx/src/os2/nsDeviceContextSpecOS2.cpp:663 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3418) gfx/src/os2/nsDeviceContextSpecOS2.cpp:664 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3419) gfx/src/os2/nsDeviceContextSpecOS2.cpp:682 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3420) gfx/src/os2/nsDeviceContextSpecOS2.cpp:683 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3421) gfx/src/os2/nsDeviceContextSpecOS2.cpp:687 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3422) gfx/src/os2/nsDeviceContextSpecOS2.cpp:697 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3423) gfx/src/os2/nsDeviceContextSpecOS2.cpp:701 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3424) gfx/src/os2/nsDeviceContextSpecOS2.cpp:897 [4] (buffer) strcat: does not check for buffer overflows. Consider using strncat or strlcat. 3425) gfx/src/os2/nsGfxDefs.cpp:32 [4] (format) printf: if format strings can be influenced by an attacker, they can be exploited. Use a constant for the format specification. 3426) gfx/src/os2/nsGfxDefs.cpp:35 [4] (format) printf: if format strings can be influenced by an attacker, they can be exploited. Use a constant for the format specification. 3427) gfx/src/os2/nsGfxDefs.cpp:131 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3428) gfx/src/photon/nsDeviceContextPh.cpp:506 [3] (buffer) getenv: Environment variables are untrustable input if they can be set by an attacker. They can have any content and length, and the same variable can be set more than once.. Check environment variables carefully before using them. 3429) gfx/src/photon/nsFontMetricsPh.cpp:175 [4] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. 3430) gfx/src/photon/nsScreenPh.cpp:56 [3] (buffer) getenv: Environment variables are untrustable input if they can be set by an attacker. They can have any content and length, and the same variable can be set more than once.. Check environment variables carefully before using them. 3431) gfx/src/ps/nsAFMObject.cpp:770 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3432) gfx/src/ps/nsAFMObject.cpp:787 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3433) gfx/src/ps/nsPostScriptObj.cpp:376 [4] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. 3434) gfx/src/ps/nsPostScriptObj.cpp:385 [2] (tmpfile) tmpfile: tmpfile() has a security flaw on some systems (e.g., older System V systems). . 3435) gfx/src/ps/nsPostScriptObj.cpp:391 [3] (tmpfile) tempnam: temporary file race condition. . 3436) gfx/src/ps/nsPostScriptObj.cpp:2087 [4] (tmpfile) system: this calls out to a new process and is difficult to use safely. try using a library call that implements the same functionality if available.. 3437) gfx/src/ps/nsPostScriptObj.cpp:2102 [4] (tmpfile) popen: this calls out to a new process and is difficult to use safely. try using a library call that implements the same functionality if available.. 3438) gfx/src/qt/nsDeviceContextSpecQT.cpp:228 [4] (format) sprintf: Potential format string problem. Make format string constant. 3439) gfx/src/qt/nsDeviceContextSpecQT.cpp:229 [4] (format) sprintf: Potential format string problem. Make format string constant. 3440) gfx/src/qt/nsDeviceContextSpecQT.cpp:236 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3441) gfx/src/qt/nsDeviceContextSpecQT.cpp:240 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3442) gfx/src/qt/nsDeviceContextSpecQT.cpp:257 [2] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. Risk is low because the source is a constant string. 3443) gfx/src/qt/nsDeviceContextSpecQT.cpp:259 [4] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. 3444) gfx/src/qt/nsFontMetricsQT.cpp:753 [3] (buffer) getenv: Environment variables are untrustable input if they can be set by an attacker. They can have any content and length, and the same variable can be set more than once.. Check environment variables carefully before using them. 3445) gfx/src/windows/nsDeviceContextSpecWin.cpp:1024 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3446) gfx/src/windows/nsFontMetricsWin.cpp:2043 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3447) gfx/src/windows/nsFontMetricsWin.cpp:3622 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3448) gfx/src/x11shared/nsFT2FontCatalog.cpp:292 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3449) gfx/src/x11shared/nsFT2FontCatalog.cpp:983 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3450) gfx/src/x11shared/nsFT2FontCatalog.cpp:1032 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3451) gfx/src/x11shared/nsFT2FontCatalog.cpp:1042 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3452) gfx/src/x11shared/nsFT2FontCatalog.cpp:1328 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3453) gfx/src/x11shared/nsFT2FontCatalog.cpp:1340 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3454) gfx/src/x11shared/nsFT2FontCatalog.cpp:2306 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3455) gfx/src/x11shared/nsFT2FontCatalog.cpp:2311 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3456) gfx/src/x11shared/nsFT2FontCatalog.cpp:2314 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3457) gfx/src/x11shared/nsFT2FontCatalog.cpp:2317 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3458) gfx/src/x11shared/nsFT2FontCatalog.cpp:2320 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3459) gfx/src/x11shared/nsFT2FontCatalog.cpp:2322 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3460) gfx/src/x11shared/nsFT2FontCatalog.cpp:2325 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3461) gfx/src/x11shared/nsFT2FontCatalog.cpp:2327 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3462) gfx/src/x11shared/nsFT2FontCatalog.cpp:2329 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3463) gfx/src/x11shared/nsFT2FontCatalog.cpp:2331 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3464) gfx/src/x11shared/nsFT2FontCatalog.cpp:2333 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3465) gfx/src/x11shared/nsFT2FontCatalog.cpp:2335 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3466) gfx/src/x11shared/nsFT2FontCatalog.cpp:2337 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3467) gfx/src/x11shared/nsFT2FontCatalog.cpp:2339 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3468) gfx/src/x11shared/nsFT2FontCatalog.cpp:2344 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3469) gfx/src/x11shared/nsFT2FontCatalog.cpp:2366 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3470) gfx/src/x11shared/nsFT2FontCatalog.cpp:2392 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3471) gfx/src/x11shared/nsFT2FontCatalog.cpp:2395 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3472) gfx/src/x11shared/nsFT2FontCatalog.cpp:2469 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3473) gfx/src/x11shared/nsX11AlphaBlend.cpp:55 [4] (format) printf: if format strings can be influenced by an attacker, they can be exploited. Use a constant for the format specification. 3474) gfx/src/xlib/nsDeviceContextSpecXlib.cpp:408 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3475) gfx/src/xlib/nsDeviceContextSpecXlib.cpp:410 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3476) gfx/src/xlib/nsDeviceContextSpecXlib.cpp:412 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3477) gfx/src/xlib/nsDeviceContextSpecXlib.cpp:414 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3478) gfx/src/xlib/nsFontMetricsXlib.cpp:4053 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3479) gfx/src/xlib/nsFontMetricsXlib.cpp:4214 [4] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. 3480) gfx/src/xlib/nsFontMetricsXlib.h:135 [4] (format) printf: if format strings can be influenced by an attacker, they can be exploited. Use a constant for the format specification. 3481) gfx/src/xlib/nsFontMetricsXlib.h:163 [4] (format) printf: if format strings can be influenced by an attacker, they can be exploited. Use a constant for the format specification. 3482) gfx/src/xlibrgb/xlibrgb.c:371 [3] (buffer) getenv: Environment variables are untrustable input if they can be set by an attacker. They can have any content and length, and the same variable can be set more than once.. Check environment variables carefully before using them. 3483) gfx/src/xlibrgb/xlibrgb.c:569 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3484) gfx/src/xlibrgb/xlibrgb.c:598 [2] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source has a constant maximum length. 3485) gfx/src/xprint/xprintutil.c:105 [3] (buffer) getenv: Environment variables are untrustable input if they can be set by an attacker. They can have any content and length, and the same variable can be set more than once.. Check environment variables carefully before using them. 3486) gfx/src/xprint/xprintutil.c:108 [3] (buffer) getenv: Environment variables are untrustable input if they can be set by an attacker. They can have any content and length, and the same variable can be set more than once.. Check environment variables carefully before using them. 3487) gfx/src/xprint/xprintutil.c:111 [3] (buffer) getenv: Environment variables are untrustable input if they can be set by an attacker. They can have any content and length, and the same variable can be set more than once.. Check environment variables carefully before using them. 3488) gfx/src/xprint/xprintutil.c:114 [3] (buffer) getenv: Environment variables are untrustable input if they can be set by an attacker. They can have any content and length, and the same variable can be set more than once.. Check environment variables carefully before using them. 3489) gfx/src/xprint/xprintutil.c:128 [3] (buffer) getenv: Environment variables are untrustable input if they can be set by an attacker. They can have any content and length, and the same variable can be set more than once.. Check environment variables carefully before using them. 3490) gfx/src/xprint/xprintutil.c:267 [4] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. 3491) gfx/src/xprint/xprintutil.c:281 [4] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. 3492) gfx/src/xprint/xprintutil.c:533 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 3493) gfx/src/xprint/xprintutil.c:683 [4] (buffer) sscanf: the scanf() family's %s operation, without a limit specification, permits buffer overflows. Specify a limit to %s, or use a different input function. 3494) gfx/src/xprint/xprintutil.c:812 [4] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. 3495) gfx/tests/coverage/nsCoverage.cpp:280 [4] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf.
|
Reporter | |
Comment 1•22 years ago
|
||
61 more flawfinder messages for gfx (4388-4448) 4388) gfx/src/beos/nsDeviceContextSpecB.cpp:295 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4389) gfx/src/beos/nsDeviceContextSpecB.cpp:296 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4390) gfx/src/beos/nsDeviceContextSpecB.cpp:299 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4391) gfx/src/beos/nsDeviceContextSpecB.cpp:335 [4] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. 4392) gfx/src/beos/nsFontMetricsBeOS.cpp:173 [4] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. 4393) gfx/src/gtk/nsDeviceContextSpecG.cpp:408 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4394) gfx/src/gtk/nsDeviceContextSpecG.cpp:410 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4395) gfx/src/gtk/nsDeviceContextSpecG.cpp:412 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4396) gfx/src/gtk/nsDeviceContextSpecG.cpp:414 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4397) gfx/src/gtk/nsFontMetricsGTK.cpp:3745 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4398) gfx/src/gtk/nsFontMetricsGTK.cpp:3872 [4] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. 4399) gfx/src/gtk/nsFontMetricsGTK.h:84 [4] (format) printf: if format strings can be influenced by an attacker, they can be exploited. Use a constant for the format specification. 4400) gfx/src/gtk/nsFontMetricsGTK.h:112 [4] (format) printf: if format strings can be influenced by an attacker, they can be exploited. Use a constant for the format specification. 4401) gfx/src/gtk/nsPrintdGTK.c:94 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4402) gfx/src/gtk/nsPrintdGTK.c:96 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4403) gfx/src/gtk/nsPrintdGTK.c:142 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4404) gfx/src/mac/nsCoreGraphicsGlue.cpp:99 [4] (format) printf: if format strings can be influenced by an attacker, they can be exploited. Use a constant for the format specification. 4405) gfx/src/mac/nsUnicodeMappingUtil.cpp:335 [4] (format) snprintf: if format strings can be influenced by an attacker, they can be exploited. Use a constant for the format specification. 4406) gfx/src/mac/nsUnicodeMappingUtil.cpp:397 [4] (format) snprintf: if format strings can be influenced by an attacker, they can be exploited. Use a constant for the format specification. 4407) gfx/src/nsNameValuePairDB.cpp:53 [4] (format) printf: if format strings can be influenced by an attacker, they can be exploited. Use a constant for the format specification. 4408) gfx/src/os2/nsDeviceContextSpecOS2.cpp:374 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4409) gfx/src/os2/nsDeviceContextSpecOS2.cpp:377 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4410) gfx/src/os2/nsDeviceContextSpecOS2.cpp:662 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4411) gfx/src/os2/nsDeviceContextSpecOS2.cpp:663 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4412) gfx/src/os2/nsDeviceContextSpecOS2.cpp:664 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4413) gfx/src/os2/nsDeviceContextSpecOS2.cpp:682 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4414) gfx/src/os2/nsDeviceContextSpecOS2.cpp:683 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4415) gfx/src/os2/nsDeviceContextSpecOS2.cpp:687 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4416) gfx/src/os2/nsDeviceContextSpecOS2.cpp:697 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4417) gfx/src/os2/nsDeviceContextSpecOS2.cpp:701 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4418) gfx/src/os2/nsDeviceContextSpecOS2.cpp:897 [4] (buffer) strcat: does not check for buffer overflows. Consider using strncat or strlcat. 4419) gfx/src/os2/nsGfxDefs.cpp:32 [4] (format) printf: if format strings can be influenced by an attacker, they can be exploited. Use a constant for the format specification. 4420) gfx/src/os2/nsGfxDefs.cpp:35 [4] (format) printf: if format strings can be influenced by an attacker, they can be exploited. Use a constant for the format specification. 4421) gfx/src/os2/nsGfxDefs.cpp:131 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4422) gfx/src/photon/nsFontMetricsPh.cpp:175 [4] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. 4423) gfx/src/ps/nsAFMObject.cpp:770 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4424) gfx/src/ps/nsAFMObject.cpp:787 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4425) gfx/src/ps/nsPostScriptObj.cpp:376 [4] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. 4426) gfx/src/ps/nsPostScriptObj.cpp:2087 [4] (tmpfile) system: this calls out to a new process and is difficult to use safely. try using a library call that implements the same functionality if available.. 4427) gfx/src/ps/nsPostScriptObj.cpp:2102 [4] (tmpfile) popen: this calls out to a new process and is difficult to use safely. try using a library call that implements the same functionality if available.. 4428) gfx/src/qt/nsDeviceContextSpecQT.cpp:228 [4] (format) sprintf: Potential format string problem. Make format string constant. 4429) gfx/src/qt/nsDeviceContextSpecQT.cpp:229 [4] (format) sprintf: Potential format string problem. Make format string constant. 4430) gfx/src/qt/nsDeviceContextSpecQT.cpp:259 [4] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. 4431) gfx/src/windows/nsDeviceContextSpecWin.cpp:1024 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4432) gfx/src/windows/nsFontMetricsWin.cpp:2043 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4433) gfx/src/windows/nsFontMetricsWin.cpp:3622 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4434) gfx/src/x11shared/nsX11AlphaBlend.cpp:55 [4] (format) printf: if format strings can be influenced by an attacker, they can be exploited. Use a constant for the format specification. 4435) gfx/src/xlib/nsDeviceContextSpecXlib.cpp:408 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4436) gfx/src/xlib/nsDeviceContextSpecXlib.cpp:410 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4437) gfx/src/xlib/nsDeviceContextSpecXlib.cpp:412 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4438) gfx/src/xlib/nsDeviceContextSpecXlib.cpp:414 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4439) gfx/src/xlib/nsFontMetricsXlib.cpp:4053 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4440) gfx/src/xlib/nsFontMetricsXlib.cpp:4214 [4] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. 4441) gfx/src/xlib/nsFontMetricsXlib.h:135 [4] (format) printf: if format strings can be influenced by an attacker, they can be exploited. Use a constant for the format specification. 4442) gfx/src/xlib/nsFontMetricsXlib.h:163 [4] (format) printf: if format strings can be influenced by an attacker, they can be exploited. Use a constant for the format specification. 4443) gfx/src/xprint/xprintutil.c:267 [4] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. 4444) gfx/src/xprint/xprintutil.c:281 [4] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. 4445) gfx/src/xprint/xprintutil.c:533 [4] (buffer) strcpy: does not check for buffer overflows. Consider using strncpy or strlcpy. 4446) gfx/src/xprint/xprintutil.c:683 [4] (buffer) sscanf: the scanf() family's %s operation, without a limit specification, permits buffer overflows. Specify a limit to %s, or use a different input function. 4447) gfx/src/xprint/xprintutil.c:812 [4] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. 4448) gfx/tests/coverage/nsCoverage.cpp:280 [4] (buffer) sprintf: does not check for buffer overflows. Use snprintf or vsnprintf.
Checked on trunk only... 3390-3391 *printf* usage is safe, debug code and big enough buffers 3392-3398) We have defined structures like this: 76 char command[ PATH_MAX ]; /* Print command e.g., lpr */ 77 char path[ PATH_MAX ]; /* If toPrinter = PR_FALSE, dest file */ 78 char printer[256]; /* Printer name */ and then we write into these like this: strcpy(mPrData.command, NS_ConvertUCS2toUTF8(command).get()); Can we be sure there is no way we could try to copy values bigger than we can handle? If we can't be sure we should either use strncpy and/or return errors, crash safely, or somehow else deal with too big values gracefully. Even if we are sure it would be nice if these methods were made safer or there was a comment explaining why they are safe (so that nobody makes them unsafe by accident - they should have seen some big comments at least). Hmm, also found a where you certainly can cause a buffer overflow, even exploitable I would think (although you would need to be able to change the users environment variable(s)): 250 if ( ( path = PR_GetEnv( "PWD" ) ) == (char *) nsnull ) 251 if ( ( path = PR_GetEnv( "HOME" ) ) == (char *) nsnull ) 252 strcpy(mPrData.path, "mozilla.ps"); 253 254 if ( path != (char *) nsnull ) 255 sprintf(mPrData.path, "%s/mozilla.ps", path); Set PWD or HOME to a value longer than PATH_MAX, and there you have it...
|
Assignee | |
Comment 3•22 years ago
|
||
Roland, can you open a bug on the PR_Env stuff?
|
||
Updated•22 years ago
|
Priority: -- → P1
Target Milestone: --- → mozilla1.3beta
|
||
Comment 4•21 years ago
|
||
Based on Mozilla1.2.1 source code, I use flawfinder to get a audit log. I add comment for each "hit" to form a audit report. About the detail of the report, please refer to it. Heikki, since it's my first audit work, can you give me some advise? Thanks very much. Rods, some hits in the audit log is hard to judge whether it's safe. Maybe you can help on these hits. Thanks a lot.
Thanks Louie, that's excellent analysis! Some comments: You say all use of PATH_MAX stuff seems to be safe, but you rely on Linux gcc. What about other platforms, some that haven't even been designed yet? What if they don't define PATH_MAX correctly? Better fix these to be absolutely safe, even in the future. Would also silence all these tools... :) gfx/src/gtk/nsDeviceContextSpecG.cpp:329 You marked undecided, I would categorize this as potentially dangerous. I don't know what printer name length could be, but in any case it would be very easy to make this safe even if name > 256, so let's make it safe. If we at any point get printer name from user, the user could write whatever they wanted, right? Also you marked several other fixed buffer sizes safe because of the current architecture. What if the architecture changes? There is nothing in Mozilla defending us from that. Better make these future proof. "constant value" is not a good description to say why something is safe, you will also need to say that the length of the constant value is ok, for example "constant value, fits in available space". It is not acceptable to mark something safe by saying "risk is low". We want 0 risk. If there is a small change of exploit, let's fix it. If there is no risk, say so. I don't think it is a good way to say something is safe by saying "static data". The code must make sure that the static data won't overflow. Since it's static, it would be good to add an assert as well so that if somebody made the static data too long by accident we would know about it. Also while checking file nsAFMObject.cpp I noticed the code does not check for out of memory conditions (for example lines 285 and 289 allocate but we never check if that succeeded or not). I think read-only open of files is still subject to race conditions. You started to open a certain file, but by the time you open it the file has been replaced and you read different data. Maybe not as dangerous as write, but still... In general, make sure your comment _really_ describes why something is safe. In general it seems like your comments are too short and cryptic for me to be sure that it really is safe. Do not automatically assume the versions of functions that take length will fix all problems. All of them will make sure not to overflow the buffer, but some of them will fail to end the string with null if input is too long (i.e. you need to explicitly set null at end). I don't remember which versions did what, unfortunately. We should update http://www.mozilla.org/projects/security/components/reviewguide.html about all the functions we use and give details on what they actually do. So, I can see you did a lot of work to dig out the X11 and GTK docs and code and everything, but in the end I think it would be much faster, simpler, and safer to just make things foolproof in Mozilla if you can't find the information in Mozilla itself. I.e., since Mozilla will not tell you what the max length of some string returned by an OS function will be, just protect Mozilla code from the possibility that the OS function could return a ridiculously long value. But having said that, it is good to say probably safe now, but we should future proof it.
|
||
Comment 6•21 years ago
|
||
Heikki Toivonen wrote: > You say all use of PATH_MAX stuff seems to be safe, but you rely on Linux gcc. > What about other platforms, some that haven't even been designed yet? > What if they don't define PATH_MAX correctly? |PATH_MAX| must be defined correctly otherwise _lots_ of software (incl. some popular shells, X11, Motif etc.) will break. As long the OS is POSIX|X/Open|Unix98-conformant |PATH_MAX| should be defined correctly, too (otherwise it will not pass the test suite==not standard-conformant). At least Solaris and AIX etc. are safe, and per loui's comment Linux is safe, too... [snip] > gfx/src/gtk/nsDeviceContextSpecG.cpp:329 You marked undecided, I would > categorize this as potentially dangerous. I don't know what printer name > length could be, but in any case it would be very easy to make this safe even > if name > 256, so let's make it safe. > If we at any point get printer name from user, the > user could write whatever they wanted, right? At least for Xprint the printer name comes from the Xprint server side which does some validation (incl. name length, which is guranteed to be less than 255 chars). In general it may be nice to use something like |nsCAutoString| for the printer name, however it should be made sure that we do not pass too long queue names to the print spooler application (in gfx/src/ps/, gfx/src/xprint is safe - see above).
|
|
||
Comment 7•21 years ago
|
||
Thanks for your specific comment for the report. Your suggestions give me great help. Below are some thoughts about the issue you mentioned. > You say all use of PATH_MAX stuff seems to be safe, but you rely on > Linux gcc.What about other platforms, some that haven't even been > designed yet? What if they don't define PATH_MAX correctly? Better > fix these to be absolutely safe, even in the future. Would also > silence all these tools... :) All the using of PATH_MAX located in "beos", "gtk", "os2", "xlib", all of that is for Unix/Linux, where PATH_MAX has definition. Considering roland's comments, the use of PATH_MAX should be safe. > gfx/src/gtk/nsDeviceContextSpecG.cpp:329 You marked undecided, I > would categorize this as potentially dangerous. I don't know what > printer name length could be, but in any case it would be very easy > to make this safe even if name 256, so let's make it safe. Here we can use strncpy instead strcpy to deal with printer name. The changes are not much. > Also you marked several other fixed buffer sizes safe because of > the current architecture. What if the architecture changes? There > is nothing in Mozilla defending us from that. Better make these > future proof. Actually, to fixed buffer size, making all "strcpy" to "strncpy" will improve the security while no destroying normal function. But that will reduce the performance a lot and also will cause too many change to current code. I think the right way is to examine which case is safe and which is not, then only fix the potential dangers. > "constant value" is not a good description > It is not acceptable to mark something safe by saying "risk is > low". > I don't think it is a good way to say something is safe by saying > "static data". > I think read-only open of files is still subject to race > conditions. Good suggestions. I will update my report according to this. > In general, make sure your comment _really_ describes why > something is safe. Sorry for that. When I went throught the code, I just write down unformal notes aside, which are arranged in the final report. I will use the form from beginning and write down more specificly next time. > Do not automatically assume the versions of functions that take > length will fix all problems. This is good suggestion. Safty should not be dependent on other module.
|
|
||
Comment 8•21 years ago
|
||
Louie Zhao:
> Actually, to fixed buffer size, making all "strcpy" to "strncpy"
> will improve the security while no destroying normal function.
Please read the manual page of |strncpy|:
-- snip --
strncpy()
The strncpy() function copies exactly n
bytes, truncating s2 or adding null characters to s1 if
necessary. The result will not be null-terminated if the
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
length of s2 is n or more. Each function returns s1.
-- snip --
AFAIK we should do anything but _not_ replacing |strcpy| with |strncpy| unless
we want to get lots of UMRs from Purify.
(I have a null-terminating version of |strncpy| around which could be
implemented in NSPR on demand (assuming that such a function is missing in
NSPR). If there is interrest, file the bug, assign it to me, I'll file the
patch...) ...Louie, switching to the versions of functions that take length as argument will of course slow that code slightly, but unless it is a really performance critical part you should not worry about it. I would do some changes, run some performance tests, and see if any optimizations etc. are needed. Like I said in comment #5, some length-taking functions that we have in the code actually do null-terminate properly, but I don't remember them at the moment. If not sure, terminate the strings explicitly. Mitch, do you have a list of functions that DO null terminate even if there was not enough space to copy the whole buffer? We should explicitly list them on http://www.mozilla.org/projects/security/components/reviewguide.html
|
|
||
Comment 10•21 years ago
|
||
Roland, "PL_strncpy" work like strncpy, "PL_strncpyz" fix the null-terminating issue of PL_strncpy. The destination string is always terminated with a '\0', unlike the traditional libc implementation.
Closing all open flawfinder bugs as WORKSFORME because we now have much better tools that do the same (well, better) kind of analysis (Coverity, Klocwork).
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•