[Edwin Calo <calo@xxxxxxxxxxxx>]

Thanks for the feedback and I made some changes that I think 
will guarantee the loop will always terminate.

(Before I was just looking at the buff_remaining...)

Now I'm keeping track of the buff_remaining, also making sure the offset
counter never exceed the original size of the packet and the offset is
always constantly increasing.

I attached the packet-postgresql.c file with the changes I made.

This is basically the loop and changes I made while walking thru 
the packet.

-got size or original buff_remaining.
while (buff_remaining > 1 && offset < size )
{
  bitone = tvb_get_ntohs (tvb, offset);
  offset += 1;

   ...
   ....  all the string stuff, etc.
   .....

  buff_remaining = tvb_length_remaining (tvb, offset);
}

Thanks.

On Sat, 2004-02-14 at 11:51, Joerg Mayer wrote:
> On Sat, Feb 14, 2004 at 05:30:41AM -0600, Edwin Calo wrote:
> > Yeah, basically I'm depending on this to walk thru the packet.
> > 
> >  buff_remaining = tvb_length_remaining (tvb, offset);
> > 
> > If the packet is malformed that will failed then or return a bogus
> > number? or probably I need to check it out more often (while walking
> > thru it?).. 
> 
> Due to the fact that I have very little time: When I first looked through
> the code, I wasn't sure whether the loop alwas terminates and deferred
> it to later (which isn't now unfortunately). We need a value that is decreased
> for every pass we make. On a superficial glance, in case of maformed packets
> the counter may remain the same (offset). The moment you guarantee that
> offset increases for every run or that the loop terminates in case that
> the offset was not increased during the loop, then everything is fine with me.
> 
>  ciao
>    Jörg

/* packet-postgresql.c
 * Routines for postgresql packet disassembly
 *
 * Copyright 2004, Edwin Calo <calo@xxxxxxxxxxxx>
 *
 * $Id: packet-loadl31-master.c,v 1.4 2004/01/20 03:41:56 jafour1 Exp $
 *
 * Ethereal - Network traffic analyzer
 * By Gerald Combs <gerald@xxxxxxxxxxxx>
 * Copyright 1998 Gerald Combs
 *
 * This program is free software; you can redistribute it and/or
 * modify it under the terms of the GNU General Public License
 * as published by the Free Software Foundation; either version 2
 * of the License, or (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License
 * along with this program; if not, write to the Free Software
 * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
 */

#ifdef HAVE_CONFIG_H
#include "config.h"
#endif

#include <glib.h>
#include <epan/packet.h>
#include <epan/strutil.h>
#include "packet-rpc.h"
#include "plugins/plugin_api.h"

static int proto_postgresql = -1;
static int hf_postgresql_response = -1;
static int hf_postgresql_request = -1;
static int hf_postgresql_length = -1;
static int hf_postgresql_string_size = -1;
static int hf_postgresql_string = -1;
static int hf_postgresql_total_length = -1;
static int hf_postgresql_bitone = -1;
static int hf_postgresql_buff_remaining = -1;
static int hf_postgresql_opcode = -1;
static int hf_postgresql_idone = -1;
static gint ett_postgresql = -1;

#define TCP_PORT_POSTGRESQL	5432


static void
dissect_postgresql (tvbuff_t * tvb, packet_info * pinfo, proto_tree * tree)
{
  proto_tree *postgresql_tree;
  proto_item *ti;
  gint offset = 0;
  gint buff_remaining = 0;
  gint size = 0;


  const guint8  *string;
  guint8 bitone;
  guint8 opcode;
  gint flag = 0;
  gint counter = 0;




  if (check_col (pinfo->cinfo, COL_PROTOCOL))
    col_set_str (pinfo->cinfo, COL_PROTOCOL, "POSTGRESQL");

  ti = proto_tree_add_item (tree, proto_postgresql, tvb, offset, -1, FALSE);
  postgresql_tree = proto_item_add_subtree (ti, ett_postgresql);

  buff_remaining = tvb_length_remaining (tvb, offset);
  size=buff_remaining;

  if (check_col (pinfo->cinfo, COL_INFO))
    {
      col_add_str (pinfo->cinfo, COL_INFO,
		   (pinfo->match_port ==
		    pinfo->destport) ? " Request" : " Response");
    }

   buff_remaining = tvb_length_remaining (tvb, offset);
   /* Used to print the initial buff remaining */
   /* if (check_col (pinfo->cinfo, COL_INFO)) { col_append_fstr (pinfo->cinfo, COL_INFO, " BuffRemainig: %d", buff_remaining ); } */

    counter=0;
    flag=0;
    while ( buff_remaining > 1 && offset < size )
    {
         bitone = tvb_get_ntohs (tvb, offset);
         offset += 1;

         if(bitone > 0x7f || (bitone > 0x0 && bitone < 0x20) ) 
         {
	    if(counter > 3)
	    {
	      if(offset > counter)
	      {
                   offset -= counter;
                   proto_tree_add_string (tree,hf_postgresql_string,tvb, offset,counter, tvb_get_ptr(tvb, offset, counter));
                   string = tvb_get_ptr (tvb, offset, counter);
		   /* Forcing end to string */
                   string[counter]='\0'; 
                   offset += counter;
                   if (check_col (pinfo->cinfo, COL_INFO)) { col_append_fstr (pinfo->cinfo, COL_INFO, " %s", string ); }
	           counter=0;
	      }
	      else
	      {
	       counter=0;
	       offset+=1;
	      }
	    }
	    else
	    {
	     counter=0;
	     offset+=1;
	    }
         }

         if( bitone == 0 )
         {
                if(counter != 0)
                { 
                  if(offset > counter)
                  {
                   offset -= counter;
		   if( counter > 1)
		   {
                    opcode = tvb_get_ntohs (tvb, offset);

                    /* An idea to take out the C in front of the UPDATE, INSERT, DELETE for now just printing everything 
		       that is on the packet */ 
                    /*
		    if (check_col (pinfo->cinfo, COL_INFO)) { col_append_fstr (pinfo->cinfo, COL_INFO, " Op: 0x%x ", opcode ); } 
		    if( opcode == 0x49 || opcode == 0x43 || opcode == 0x62 || opcode == 0x55)
		    {
		      offset+=1;
		      counter-=1;
		    }
		    */

                    proto_tree_add_string (tree,hf_postgresql_string,tvb, offset,counter, tvb_get_ptr(tvb, offset, counter));
                    string = tvb_get_ptr (tvb, offset, counter);
		   /* Forcing end to string */
                    string[counter]='\0'; 
                    if (check_col (pinfo->cinfo, COL_INFO)) { col_append_fstr (pinfo->cinfo, COL_INFO, " %s", string ); }
                   }
		   offset += counter;
                  }
                  counter = 0;
                }
                counter=0;
         }
         else
         {
             counter += 1;
         }

         buff_remaining = tvb_length_remaining (tvb, offset);
     }
} 


void proto_register_postgresql (void)
{

  static hf_register_info hf[] = {
    {&hf_postgresql_response,
     {"Response", "postgresql.response",
      FT_BOOLEAN, BASE_NONE, NULL, 0x0,
      "TRUE if postgresql response", HFILL}},
    {&hf_postgresql_request,
     {"Request", "postgresql.request",
      FT_BOOLEAN, BASE_NONE, NULL, 0x0,
      "TRUE if postgresql request", HFILL}},
    {&hf_postgresql_string, {"String", "hf_postgresql_string", FT_STRING, BASE_NONE, NULL, 0x0, "", HFILL}},
    {&hf_postgresql_length, {"Length", "hf_postgresql_length", FT_UINT32, BASE_DEC, NULL, 0x0, "", HFILL}},
    {&hf_postgresql_string_size, {"Size", "hf_postgresql_string_size", FT_UINT32, BASE_DEC, NULL, 0x0, "", HFILL}},
    {&hf_postgresql_total_length, {"TotalLength", "hf_postgresql_total_length", FT_UINT16, BASE_DEC, NULL, 0x0, "", HFILL}},
    {&hf_postgresql_buff_remaining, {"Buffer Remaining", "hf_postgresql_buff_remaining", FT_UINT8, BASE_DEC, NULL, 0x0, "", HFILL}},
    {&hf_postgresql_opcode, {"Op Code", "hf_postgresql_opcode", FT_UINT32, BASE_HEX, NULL, 0x0, "", HFILL}},
    {&hf_postgresql_bitone, {"Bitone", "hf_postgresql_bitone", FT_UINT8, BASE_HEX, NULL, 0x0, "", HFILL}},
    {&hf_postgresql_idone, {"idone", "hf_postgresql_idone", FT_UINT8, BASE_HEX, NULL, 0x0, "", HFILL}},
 


  };

  static gint *ett[] = {
    &ett_postgresql,
  };

  proto_postgresql =
    proto_register_protocol ("POSTGRESQL", "POSTGRESQL", "postgresql");
  proto_register_field_array (proto_postgresql, hf, array_length (hf));
  proto_register_subtree_array (ett, array_length (ett));
}

void
proto_reg_handoff_postgresql (void)
{
  dissector_handle_t postgresql_handle;

  postgresql_handle =
    create_dissector_handle (dissect_postgresql, proto_postgresql);
  dissector_add ("tcp.port", TCP_PORT_POSTGRESQL, postgresql_handle);
}