I plead "insufficient data" to solve your problem. There's too much left out of the code. Let me comment on the code to indicate why I think so. In what follows, I have adjusted the indentation to match what I'm used to. It helps me to understand the flow.

##################
sub reordermenu {
##################

  accesscheck();

  @menuitems = Cobra::chomp_database("$datadir/defaults/menu.dat", "0"
+);

  open (DATABASE,"<$datadir/defaults/menu.dat");
  hold (DATABASE);
  @menuitems = <DATABASE>;
  release (DATABASE);
  close (DATABASE);
[download]

There's a lot missing here. access_check(), hold() and release() all seem to have obvious meanings, but what the heck does Cobra::chomp_database() do in relation to what follows? It probably doesn't matter, though.

 MENUITEM: for ($ndx=0; $ndx<= $#menuitems; $ndx++) {
    ($fid,$fmenu_type,$fmenu_show,$fmenu_filename,
      $fmenu_name,$fmenu_icon)=split(/\|/,$menuitems[$ndx]);
    if ($fid eq $info{'m_id'}) {
      last MENUITEM;
    }
  }
[download]

The MENUITEM label is superfluous, since there's only the one loop. The if block could be recast in typical Perl postfix style as:

    last if ($fid eq $info{'m_id'});
[download]

Next we have:

  $swap=false;
  if ($m_action eq "moveup" && $ndx != 0) {
    $tondx = $ndx - 1;
    $swap=true;
    
  } elsif ($m_action eq "movedown" && $ndx != $#menuitems) {
    $tondx = $ndx + 1;
    $swap=true;
  }
[download]

OK, where does "$m_action" come from? Do we really just swap once in this sub? Are you loading the entire database and saving it for just one swap? Something seems to be missing.