View Issue Details

IDProjectCategoryView StatusLast Update
0000498LDMud 3.3Efunspublic2009-03-10 16:25
Reporterfippo Assigned Tozesstra  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.3.713 
Target Version3.3.719Fixed in Version3.3.719 
Summary0000498: idna_stringprep doesn't work
DescriptionMissing free_svalue...

I wonder why I did not notice that earlier...
TagsNo tags attached.

Activities

2007-01-17 03:57

 

pkg-idna.patch (480 bytes)   
Index: pkg-idna.c
===================================================================
--- pkg-idna.c	(revision 2314)
+++ pkg-idna.c	(working copy)
@@ -144,6 +144,7 @@
         if (argflags & STRINGPREP_NO_NFKC_FLAG) flags |= STRINGPREP_NO_NFKC;
         if (argflags & STRINGPREP_NO_BIDI_FLAG) flags |= STRINGPREP_NO_BIDI;
         if (argflags & STRINGPREP_NO_UNASSIGNED_FLAG) flags |= STRINGPREP_NO_UNASSIGNED;
+	free_svalue(sp--);
     }
 
     /* Get and check the profile */
pkg-idna.patch (480 bytes)   

zesstra

2009-03-04 16:56

administrator   ~0000974

Gna. And I wonder, why nobody seems to have noticed this issue until now.
The third argument does not need a free_svalue(), because it is just a number, but the stackpointer has to be decreased.
BTW: I think, the first argument (string) is not freed as well.

2009-03-09 15:00

 

0001-Fixed-freeing-of-the-arguments-of-f_idna_stringprop.patch (1,539 bytes)   
From dc9a1490885efeea0e3a5946f68651fe99ade2fc Mon Sep 17 00:00:00 2001
From: zesstra <zesstra@zesstra.de>
Date: Mon, 9 Mar 2009 19:58:36 +0100
Subject: [PATCH] Fixed freeing of the arguments of f_idna_stringprop().
 The stack pointer was decremented after getting the third argument.
 Converted free'ing of the second argument into stack pointer decrement.
 Added free'ing of the given string.
 Removed putting a return value on the stack in case of calling errorf.

---
 src/pkg-idna.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/pkg-idna.c b/src/pkg-idna.c
index c0ece1a..9bf7f4f 100644
--- a/src/pkg-idna.c
+++ b/src/pkg-idna.c
@@ -130,6 +130,7 @@ f_idna_stringprep (svalue_t *sp)
     /* Get and check the flags. */
     {
         p_uint argflags = (p_uint)sp->u.number;
+        sp--;
 
         if (argflags > (STRINGPREP_FLAG_MAX << 1)-1
          || argflags & (STRINGPREP_NO_NFKC_FLAG | STRINGPREP_NO_BIDI_FLAG)
@@ -148,7 +149,7 @@ f_idna_stringprep (svalue_t *sp)
 
     /* Get and check the profile */
     prof = (int)sp->u.number;
-    free_svalue(sp--);
+    sp--;
 
     /* select the profile */
     switch(prof)
@@ -206,12 +207,13 @@ f_idna_stringprep (svalue_t *sp)
 
     if (ret != STRINGPREP_OK)
     {
-        put_number(sp, -ret);
         errorf("stringprep(): Error %s", stringprep_strerror(ret));
         /* NOTREACHED */
-    } 
+    }
     else 
     {
+        // free the string argument
+        free_svalue(sp);
         put_c_string(sp, buf);
     }
 
-- 
1.6.1

zesstra

2009-03-09 15:02

administrator   ~0000984

I modified fippos patch a little bit and changed the two free_svalue of T_NUMBER arguments into sp--. Additionally I added free'ing of the argument string before putting the result string on the stack and removed putting a number on the stack before calling errorf(), because it is anyway without effect.

My system does not support IDNA right now. Could somebody please review/test the attached patch? ;-)

fippo

2009-03-10 07:35

reporter   ~0000986

Last edited: 2009-03-10 07:35

Minor bug: you decrement the sp in the beginning of the argflags block, but it is decremented again at the end of the block.
Seems to be working after fixing that. Otoh, it was worked for me before :-)

p.s.: we don't use that stuff at all since it was renamed to idna_stringprep and our preprocessor directives expect stringprep, silently falling back to lower_case if there is no such efun...

zesstra

2009-03-10 08:20

administrator   ~0000987

Mhmm. I am confused. I don't have a double decrement in the argflags block, there is only the one at the beginning. Did you apply both patches?

fippo

2009-03-10 08:31

reporter   ~0000988

psyclpc has the first patch already applied (which has a free_svalue(sp--) at the end), which lead to the double decrement when applying your patch.

zesstra

2009-03-10 16:25

administrator   ~0000989

This should then be fixed in r2525.

Issue History

Date Modified Username Field Change
2007-01-17 03:57 fippo New Issue
2007-01-17 03:57 fippo File Added: pkg-idna.patch
2009-03-04 16:56 zesstra Note Added: 0000974
2009-03-04 16:56 zesstra Status new => confirmed
2009-03-04 16:56 zesstra Project LDMud => LDMud 3.3
2009-03-04 16:57 zesstra Projection none => minor fix
2009-03-04 16:57 zesstra Target Version => 3.3.719
2009-03-09 14:59 zesstra Status confirmed => assigned
2009-03-09 14:59 zesstra Assigned To => zesstra
2009-03-09 15:00 zesstra File Added: 0001-Fixed-freeing-of-the-arguments-of-f_idna_stringprop.patch
2009-03-09 15:02 zesstra Note Added: 0000984
2009-03-09 15:02 zesstra Status assigned => feedback
2009-03-10 07:35 fippo Note Added: 0000986
2009-03-10 07:35 fippo Note Edited: 0000986
2009-03-10 08:20 zesstra Note Added: 0000987
2009-03-10 08:31 fippo Note Added: 0000988
2009-03-10 16:25 zesstra Note Added: 0000989
2009-03-10 16:25 zesstra Status feedback => resolved
2009-03-10 16:25 zesstra Fixed in Version => 3.3.719
2009-03-10 16:25 zesstra Resolution open => fixed