#141 DSPAM broken after upgrade to PostgreSQL 9.1

v3.10.x
pending
None
5
2014-08-20
2011-09-26
Ingo Theiss
No

After PostgreSQL 9.1 has hit Debian/testing DSPAM fails to insert data into the table 'dspam_signature_data' (see attached sql_erros.txt). After digging around in the source code I have seen the function 'PQescapeByteaConn' (pgsql_drv.c line 1574) is used to escape the data. Unfortunately with PostgreSQL 9.1 the default from the setting 'standard_conforming_strings' has changed from 'off' to 'on'. Please read the following thread as I am not an expert in C-Programming nor in PostgreSQL: http://postgresql.1045698.n5.nabble.com/PQescapeByteaConn-returns-wrong-string-for-PG9-1-Beta3-td4667713.html

I can confirm that DSPAM is working again if I set 'standard_conforming_strings = off' in my postgresql.conf. But since the new default for PostgreSQL 9.1 is 'standard_conforming_strings = on' this should be considered in pgsql_drv.c.

Discussion

  • Ingo Theiss

    Ingo Theiss - 2011-09-26
     
  • Stevan Bajic

    Stevan Bajic - 2011-10-01

    Hello Ingo, can you try latest GIT version of DSPAM and tell me if the problem still exists there?

     
  • Stevan Bajic

    Stevan Bajic - 2011-10-01
    • assigned_to: nobody --> sbajic
     
  • Ingo Theiss

    Ingo Theiss - 2011-10-03

    Hi Stevan,

    so far I have used the Debian packages from Julien Valroff (http://www.kirya.net/) which are available from the official Debian Backports repository. I will try to compile the pgsql part and the if problem still exists.

    Regards,

    Ingo

     
  • Stevan Bajic

    Stevan Bajic - 2011-11-02
    • status: open --> pending
     
  • The Duck

    The Duck - 2012-12-12

    I got warnings with this fix and standard_conforming_strings to off and this resulted in DSPAM returning with a non-zero exit code and mail delivery failure. Maybe seeting escape_string_warning to off would help, as it warns if any backslash is found in the string, even if doubled.
    As i can now switch standard_conforming_strings to on, i just did, then i should be able to ping this bug if the fix is not enough.

     
  • Stevan Bajic

    Stevan Bajic - 2013-11-26

    Hello Thomas,

    thank you for the patch. Are you sure it will fix your issues with strings? Inside the patch I see that you are prefixing the signature. Why? The signature it self is simple and does not have any backslash or so in it. IMHO the E in front the signature is not required. Does you patch work with standard_conforming_strings set to on and off too?

    cheers,

    Stevan

     
    • The Duck

      The Duck - 2013-11-26

      Coin,

      Thomas's reasoning is right, see:
      http://www.postgresql.org/docs/current/static/runtime-config-compatible.html

      The E'' syntax was introduced in 8.1, and i don't think there's any
      interest in supporting older version in DSPAM.

      Quoting Stevan Bajic sbajic@users.sf.net:

      IMHO the E in front the signature is not required.

      For the signature there is no need for it if you don"t use backslashes
      to escape, you're right.

      Does you patch work with standard_conforming_strings set to on and off too?

      If it is on, you must use E'' when you use backlashes to escape.
      If it is off, you may use E'' but it will still work fine with a
      warning (if not disabled with escape_string_warning set to false).

      Regards.

      --
      Marc Dequènes (Duck)

       
      • Thomas Preud'homme

        Yes, exactly. The E prefix indicates to PostgreSQL that it is a string constant with escaping. In the absence of E prefix, it is a normal string constant. standard_conforming_strings only impact the way normal string constant are interpreted by PostgreSQL. When set to on, it will interpret every character literally, including the backslash. That is, the backslash is part of the string like any letter from a to z would be and does not introduce an escape sequence. When standard_conforming_strings is set to off, PostgreSQL will accept escape sequence in normal string constants. Prefixing with E means it will accept escape sequence in the next string constant, no matter what. If there is no escape sequence in a given string constant, then prefixing or not with E wouldn't change anything.

        With regards to my patch, I didn't check the semantic of the strings constant, that is why I prefixed all of them with E. As explained above, it will work no matter what is the value of standard_conforming_strings. However if signature never contains characters that needs to be escaped, then the E prefix is useless (but not harmful).

        I hope I answered your question.

        Best regards,

        Thomas

         
  • Thomas Preud'homme

    Oh and also in the code you already call function to escape signature so it was only consistent for me to add the E prefix. If you know these strings don't need to be escape, then you probably should remove the call PQescapeStringConn for these constant strings.

     
  • Stevan Bajic

    Stevan Bajic - 2013-11-27

    Hello Thomas,

    the signature does not contain any special characters. So the E is not needed. Anyway.... if the E is not needed im the signature then I don't see how your patch is going to fix issues with escaping strings? The initial problem reported by Deboan folks still exists then. Right?

    Stevan

     
    • Thomas Preud'homme

      TL;DR: not only signature are escaped + what is the content of SIG->data in line 1524 of pgsql_drv.c?

      Great, that shorten the patch then. As you can see I changed all the string constant and not all of them seems to be about signature. For instance the 5th and 6th change in the patch are about a name (SELECT ... WHERE virtual_name = name_esc). If the name comes from the user it could contain some character that needs to escape. Take my last name for example (Preud'homme): the apostrophie that it contains needs to be escaped.

      I'm less sure for the 7th and 8th changes in the patch as it is about the preferences. But if it's exactly what is written in the configuration file (I mean not filtered already), then it might contains any weird character as well.

      As to the bug experienced in Debian, it is the second part of second change. It's in the case of SIG->data contains escape sequence. Indeed, it looks like a signature so there might be something weird. I'd appreciate if you can tell me what this data can contain. It might be a mistake I made as I just realized when doing a new upload that at some point I carried over a patch I made for dspam 3.10.1 in Debian to set standard_conforming_strings to off to dspam 3.10.2. Since a lot of code was added in 3.10.2 to escape strings, maybe a string was too escaped or something and that corrupted the database of a user. I'm not sure, I'd need to look more into it. Please let me know what SIG->data should contain and if it is abnormal that it contains an escape sequence.

      Best regards,

      Thomas

       
      • Thomas Preud'homme

        Hi Stevan,

        after a new bug in Debian which I think is related to my latest change, I am trying to better understand what is going on. First of all, I am now sure that the bug we got in Debian [0] was indeed about a missing E prefix. If I understood correctly, SIG->data is the data associated to a signature and is a raw sequence of byte (bytea in postgresql terminology). Therefore, it needs to be escaped (as dspam currently does) and is very likely to contain escape sequence (by definition). Thus, it needs to be prefixed by E.

        [0] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=719663

        As to the signature, you indeed control how it looks like when you create it. But then, when you receive an email, you parse the header which contains the signature and you look if it exists in the database. Since it is an incoming email, it could be a forged signature that contains escape sequence. This could be made in an attempt to fill the /var/log partition of a dspam setup in order to create a DOS. So I believe except for the insertion of the signature and its associated data, all the other place where dspam manipulate a signature it might contains some escape sequence and should thus be prefixed with a E again.

        I'm probably not teaching you anything since you correctly called PQescapeStringConn for the signature before doing any SQL request so you knew it could contains weird character. It is just the normal consequence to also prefix the escaped signature with a capital E, or postgresql could complain. I didn't investigate the other place where my patch adds a E prefix but it might be justified by following the same reasonning. Anyway, as explained before, adding the capital E doesn't hurt, unless you have a string that contains backslashes that you want to store as is.

        I hope this email is clearer and more complete than the previous one.

        Best regards,

        Thomas

         
        • Thomas Preud'homme

          My bad, the E prefix is indeed not necessary. PQescapeStringConn and PQescapeByteaConn does less escaping when standard_conforming_strings is on so that the result of calling these escaping function can be passed as a normal string litteral (ie no need for E prefix which indicates escaped strings). We had the bug in Debian because a wrong merge pulled a patch to set standard_conforming_strings to off.

          In other words, on our side (Debian) everything is fine and this bug can be closed (it is closed in Debian).

           

Log in to post a comment.