GNU bug report logs - #73985
fix: charset.max_char may be used before being set

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 73985 in the body.
You can then email your comments to 73985 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: altermo31 <altermo31 <at> proton.me>
To: "bug-gnu-emacs <at> gnu.org" <bug-gnu-emacs <at> gnu.org>
Subject: fix: charset.max_char may be used before being set
Date: Thu, 24 Oct 2024 11:46:17 +0000

[Message part 1 (text/plain, inline)]

Problem:
If the value to be set to charset.max_char is invalid, an error occurs, but the error uses the yet unset charset.max_char.

Solution:
Use the value that charset.max_char would be set to if the value wasn't invalid.

[0001-fix-charset.max_char-may-be-used-before-being-set.patch (text/x-patch, attachment)]

Message #8 received at 73985 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: altermo31 <altermo31 <at> proton.me>
Cc: 73985 <at> debbugs.gnu.org
Subject: Re: bug#73985: fix: charset.max_char may be used before being set
Date: Thu, 24 Oct 2024 17:12:13 +0300

> Date: Thu, 24 Oct 2024 11:46:17 +0000
> From:  altermo31 via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> Problem:
> If the value to be set to charset.max_char is invalid, an error occurs, but the error uses the yet unset charset.max_char.
> 
> Solution:
> Use the value that charset.max_char would be set to if the value wasn't invalid.

Thanks.  You are right that the original code is incorrect, but the
code you propose has a subtle problem: the sum

  i + charset.code_offset

could overflow.  So we need to find a safer way of explaining the
problem.

Message #11 received at 73985 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: altermo31 <at> proton.me
Cc: 73985 <at> debbugs.gnu.org
Subject: Re: bug#73985: fix: charset.max_char may be used before being set
Date: Thu, 24 Oct 2024 17:42:36 +0300

> Cc: 73985 <at> debbugs.gnu.org
> Date: Thu, 24 Oct 2024 17:12:13 +0300
> From: Eli Zaretskii <eliz <at> gnu.org>
> 
> > Date: Thu, 24 Oct 2024 11:46:17 +0000
> > From:  altermo31 via "Bug reports for GNU Emacs,
> >  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> > 
> > Problem:
> > If the value to be set to charset.max_char is invalid, an error occurs, but the error uses the yet unset charset.max_char.
> > 
> > Solution:
> > Use the value that charset.max_char would be set to if the value wasn't invalid.
> 
> Thanks.  You are right that the original code is incorrect, but the
> code you propose has a subtle problem: the sum
> 
>   i + charset.code_offset
> 
> could overflow.  So we need to find a safer way of explaining the
> problem.

I fixed that (on the master branch) like this:

diff --git a/src/charset.c b/src/charset.c
index e8d0826..f7d80cc 100644
--- a/src/charset.c
+++ b/src/charset.c
@@ -1007,7 +1007,8 @@ DEFUN ("define-charset-internal", Fdefine_charset_internal,
 
       i = CODE_POINT_TO_INDEX (&charset, charset.max_code);
       if (MAX_CHAR - charset.code_offset < i)
-	error ("Unsupported max char: %d", charset.max_char);
+	error ("Unsupported max char: %d + %ud > MAX_CHAR (%d)",
+	       i, charset.max_code, MAX_CHAR);
       charset.max_char = i + charset.code_offset;
       i = CODE_POINT_TO_INDEX (&charset, charset.min_code);
       charset.min_char = i + charset.code_offset;

Message #16 received at 73985-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: altermo31 <at> proton.me
Cc: 73985-done <at> debbugs.gnu.org
Subject: Re: bug#73985: fix: charset.max_char may be used before being set
Date: Sat, 09 Nov 2024 11:12:22 +0200

> Cc: 73985 <at> debbugs.gnu.org
> Date: Thu, 24 Oct 2024 17:42:36 +0300
> From: Eli Zaretskii <eliz <at> gnu.org>
> 
> > Thanks.  You are right that the original code is incorrect, but the
> > code you propose has a subtle problem: the sum
> > 
> >   i + charset.code_offset
> > 
> > could overflow.  So we need to find a safer way of explaining the
> > problem.
> 
> I fixed that (on the master branch) like this:
> 
> diff --git a/src/charset.c b/src/charset.c
> index e8d0826..f7d80cc 100644
> --- a/src/charset.c
> +++ b/src/charset.c
> @@ -1007,7 +1007,8 @@ DEFUN ("define-charset-internal", Fdefine_charset_internal,
>  
>        i = CODE_POINT_TO_INDEX (&charset, charset.max_code);
>        if (MAX_CHAR - charset.code_offset < i)
> -	error ("Unsupported max char: %d", charset.max_char);
> +	error ("Unsupported max char: %d + %ud > MAX_CHAR (%d)",
> +	       i, charset.max_code, MAX_CHAR);
>        charset.max_char = i + charset.code_offset;
>        i = CODE_POINT_TO_INDEX (&charset, charset.min_code);
>        charset.min_char = i + charset.code_offset;

No further comments, so I'm now closing this bug.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.