Open Bug 1246768 Opened 9 years ago Updated 2 years ago

Use unsigned char with ctype.h functions

Categories

(NSS :: Build, defect)

Unspecified
NetBSD
defect

Tracking

(firefox47 fixed)

REOPENED
Tracking Status
firefox47 --- fixed

People

(Reporter: coypu, Unassigned)

Details

Attachments

(2 files)

Attached file use-unsigned-char-ctype (deleted) —
User Agent: Mozilla/5.0 (X11; NetBSD amd64; rv:44.0) Gecko/20100101 Firefox/44.0 Build ID: 20160207041628 Actual results: NSS should use unsigned char when using functions from ctype.h Please see the following page (man 3 isdigit from Linux): http://linux.die.net/man/3/isdigit To quote, "These functions check whether c, which must have the value of an unsigned char". This is a patch to cast to unsigned char on every such comparison.
There may be more in the tests.
OS: Unspecified → NetBSD
https://hg.mozilla.org/integration/mozilla-inbound/rev/d8781a4c7092db380f52de8467515fd317e24201 Bug 1246768 - part 1: argument conversion for Atomics.isLockFree in runtime. r=bbouvier
Status: UNCONFIRMED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
(In reply to Lars T Hansen [:lth] from comment #2) > https://hg.mozilla.org/integration/mozilla-inbound/rev/ > d8781a4c7092db380f52de8467515fd317e24201 > Bug 1246768 - part 1: argument conversion for Atomics.isLockFree in runtime. > r=bbouvier (In reply to Phil Ringnalda (:philor) from comment #3) > https://hg.mozilla.org/mozilla-central/rev/d8781a4c7092 These were actually for Bug 1246767.
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: FIXED → ---

I went through and made this patch independently before I came upon this open bug. Note: The misuse of ctype(3) can lead to undefined behaviour, including crashes and/or exposure of secrets from reading before the beginning of a ctype table in memory, and the problem is not limited to NetBSD. From the commit message I wrote:

The ctype(3) functions take arguments of type int that are either

(a) EOF, or
(b) unsigned char values, {0, 1, 2, ..., 255} if char is 8-bit.

Passing values of type char, on platforms where it is signed, can go
wrong -- negative values may be confused with EOF (typically -1) or
may lead to undefined behaviour ranging in practice from returning
garbage data (possibly out of an adjacent buffer in memory that may
contain secrets) to crashing with SIGSEGV (if the page preceding the
ctype table is unmapped).

The ctype(3) functions can't themselves convert to unsigned char
because then they would give the wrong answers for EOF, for use with
functions like getchar and fgetc; the user has to cast char to
unsigned char.

It is possible, in some cases, to prove that the input always
actually lies in {0, 1, 2, ..., 127}, so the conversion to unsigned
char is not necessary. I didn't check whether this was the case --
it's safer to just adopt the habit of always casting char to unsigned
char first before using the ctype(3) macros, which satisfies a
compiler warning on some systems designed to detect this class of
application errors at compile-time.

Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: