Skip to content
Snippets Groups Projects
  1. Apr 10, 2021
    • Simon Tatham's avatar
      dialog system: add a side-by-side alignment feature. · 1276c13e
      Simon Tatham authored
      This will let us put two controls side by side (e.g. in disjoint
      columns of a multi-col layout) and indicate that instead of the
      default behaviour of aligning their top edges, their centreline (or,
      even better if available, font baseline) should be aligned.
      
      NFC: nothing uses this yet.
      1276c13e
    • Simon Tatham's avatar
      gtkwin: remove a redundant test in delete_window. · d33f889a
      Simon Tatham authored
      We never expect to be passed a NULL GtkFrontend pointer, and even if
      we were, we'd have crashed several lines above this test.
      
      It was benign, of course, but Coverity (which pointed it out) dislikes
      this kind of thing on the basis that it's confusing - you ought to
      either test it for NULL properly, or not at all - and I see its point.
      d33f889a
    • Simon Tatham's avatar
      winctrls: fix warning about uninitialised variable. · 597e4731
      Simon Tatham authored
      Coverity points out that it's theoretically possible for the main loop
      in radioline_common() to read r.bottom without having gone through the
      conditional setup at the start of the function _or_ a previous
      iteration of the main loop. I think this can only happen in some silly
      case that doesn't actually come up, but on the other hand, it's easy
      to add the necessary robustness.
      597e4731
    • Simon Tatham's avatar
      Fix failure handling when loading a PPK file. · ed1d64b4
      Simon Tatham authored
      Coverity pointed out that I'd checked if the LoadedFile was NULL, set
      an error message ... and then accidentally fallen through to the
      success handler anyway.
      ed1d64b4
    • Simon Tatham's avatar
      pageant_get_keylist: fix handling of bad SSH-1 data from agent. · d53b3bcd
      Simon Tatham authored
      Coverity points out that if rsa_ssh1_public_blob_len sees data it
      doesn't like, it returns -1 to indicate an error. But the code that
      uses it to parse the SSH1_AGENT_RSA_IDENTITIES_ANSWER payload was
      passing it directly to get_data() as a length field, without checking
      for that case. Now we do check it, and use it to set the existing
      kl->broken flag that indicates that the key list was not correctly
      formatted.
      d53b3bcd
    • Simon Tatham's avatar
      Impose an upper bound on incoming SFTP packet length. · edadfa70
      Simon Tatham authored
      Coverity was unhappy that I'd used the packet length as a loop bound
      without sanitising it first (on the basis that it had decided anything
      coming from GET_32BIT_MSB_FIRST was potentially tainted).
      
      I think this is not a security issue: all that will happen if the
      server sends a huge packet length is that we'll try to allocate space
      for it. On a 64-bit machine we might even _succeed_; on 32-bit, we'll
      fail, and snewn() will abort the program rather than return NULL. So
      *technically* this is a remote-triggered crash. But it can only happen
      in a situation where the same server could have triggered the
      termination of the SFTP connection just as easily by simply closing it
      - the only difference is that the client would die with a different
      fatal error message.
      
      (In particular, it isn't even a DoS against other processes
      participating in a connection-shared SSH session. The upstream will
      pass the SFTP data stream through without parsing it, so it and the
      other downstreams will be unaffected. Only the particular downstream
      operating the SFTP client will run into this problem.)
      edadfa70
    • Simon Tatham's avatar
      winpgntc: fix mishandling of named-pipe errors. · 165f630a
      Simon Tatham authored
      If named_pipe_agent_gotdata was called with an error or EOF status, it
      would call agent_cancel_query(pq), but then accidentally fall through
      to the non-error handler which would dereference pq. I meant to return
      early in that situation, and Coverity spotted that I'd left out the
      early return statement.
      165f630a
    • Simon Tatham's avatar
      fc8550c0
    • Simon Tatham's avatar
      unifontsel: add extra double-checks of fontinfo values. · c5724c46
      Simon Tatham authored
      Coverity objected to several similar cases in this code in which I'd
      checked a pointer for NULL after already having done things to it. I
      think all the cases are benign, in that (as the comments tersely
      mention) those checks could only fail if the unifontsel system had got
      _really_ confused, in which case probably some other bug would have
      been on the point of manifesting anyway. But Coverity has a point
      anyway: if I'm _going_ to check those values for NULL, let's check
      them consistently.
      c5724c46
    • Simon Tatham's avatar
      gtkwin: remove dead code in cut buffer handling. · 525b767c
      Simon Tatham authored
      Commit d851df48 deleted a #if / #else / #endif on the grounds
      that the condition would now always be true, without also deleting the
      code inside the #else. Happily, the then-branch ended with a return,
      so it was a benign mistake - the erroneously left-in else-clause code
      was unreachable. But now Coverity has pointed it out, let's remove it.
      525b767c
    • Simon Tatham's avatar
      Argon2 hprime: remove pointless bounds check. · 52fa23c7
      Simon Tatham authored
      Coverity points out that we don't need to check the output buffer
      bound before writing out the first 32 bytes of each full-length
      BLAKE2b invocation, because the only time we're doing a full-length
      one in the first place is if the output buffer bound was at least 64
      bytes.
      
      (More specifically: whenever we're in the while loop, length > 64, so
      setting chunk = 32 and then checking if chunk > length has a totally
      predictable answer.)
      52fa23c7
  2. Apr 09, 2021
    • Simon Tatham's avatar
      Fixes from an attempted winelib build. · bb59f273
      Simon Tatham authored
      The winelib headers don't have GWL_foo, only GWLP_foo (which, fair
      enough, I should have been using already). And a side effect was to
      point out some slightly incautious integer types in printf argument
      lists.
      bb59f273
  3. Apr 08, 2021
    • Simon Tatham's avatar
      winplink: create an Ldisc for the backend to use. · 5c051f00
      Simon Tatham authored
      This has apparently been missing more or less forever (though Unix
      Plink does have it). Without this, ssh.c can't call ldisc_update,
      which can't pass the current editing and echoing settings through to
      seat_echoedit_update. Windows Plink has always _had_ an implementation
      of that seat method (and the static function that preceded it), but it
      was never able to be called, because of that missing link.
      
      The result was that manual overrides in the Conf to force local
      editing/echoing to a particular state were not honoured by Windows
      Plink, and neither were mainchan.c's attempts to set the state
      automatically based on whether a pty had been allocated at the far end
      of the connection.
      5c051f00
    • Jacob Nevins's avatar
      psocks: remove print_c_string(). · ba599bf5
      Jacob Nevins authored
      We already had one of these in utils, since c18e5dc8.
      ba599bf5
    • Jacob Nevins's avatar
      Man page for psocks. · f039e5f4
      Jacob Nevins authored
      f039e5f4
    • Jacob Nevins's avatar
      7d1aae13
  4. Apr 07, 2021
    • Jacob Nevins's avatar
      cmdgen: have --dump output private parts of PPKs. · af9a66be
      Jacob Nevins authored
      This seems more useful than the previous behaviour of not prompting for
      a passphrase and only emitting the public part; if we want that back
      I suppose we could invent a "-O text-public".
      
      Also, document the text dump format a bit in the man page.
      af9a66be
    • Simon Tatham's avatar
      winpgnt: fix crash if deferred-decryption passphrase is wrong. · 21c2e451
      Simon Tatham authored
      Thanks to Jacob for spotting this one: when we hand a passphrase back
      to pageant.c via pageant_passphrase_request_success(), if the key
      doesn't decrypt successfully, pageant.c responds by immediately
      issuing another passphrase prompt - and it does it _synchronously_, by
      calling back from within pageant_passphrase_request_success(). In this
      case, the effect is that we end up in ask_passphrase_common(), which
      starts by asserting that nonmodal_passphrase_hwnd is NULL - but it
      wasn't NULL _quite_ yet, because end_passphrase_dialog() was expecting
      to clean it up immediately after pageant_passphrase_request_success()
      returned, i.e. just too late.
      
      The heavyweight fix would be to arrange a toplevel callback to defer
      opening the new window until after the old one had been cleaned up.
      But in this case I don't think there's any need: it's enough to simply
      do the operations in end_passphrase_dialog() in the opposite order, so
      that first we destroy the old window and set nonmodal_passphrase_hwnd
      back to NULL, and _then_ we call into pageant.c which might call us
      back and open a fresh window.
      21c2e451
    • Jacob Nevins's avatar
      Document a couple of new cmdgen features. · 1069ba6a
      Jacob Nevins authored
      "-O text" (c18e5dc8) and ability to read key from stdin (a7599a57a3).
      1069ba6a
    • Jacob Nevins's avatar
      725a0aba
  5. Apr 06, 2021
  6. Apr 05, 2021
  7. Apr 04, 2021
    • Simon Tatham's avatar
      winpgnt: grey out key-list window buttons as appropriate. · 8edeecdc
      Simon Tatham authored
      Now the Remove button is disabled if there aren't any keys at all
      loaded, and the Re-encrypt button is disabled if no key is currently
      in a state where it's decrypted but re-encryptable.
      8edeecdc
    • Simon Tatham's avatar
      winpgnt: menu options to delete/reencrypt everything. · b8374f1b
      Simon Tatham authored
      Now the systray menu includes 'Remove All Keys' and 'Re-encrypt All
      Keys' options, which do exactly what they say on the tin.
      b8374f1b
    • Simon Tatham's avatar
      winpgnt: fix accidental bisection of menu id definitions. · 39a72c16
      Simon Tatham authored
      Not quite sure how that happened! But at some point in the past, a bunch
      of other definitions in winpgnt.c managed to get in between the first
      few IDM_FOO constants and the last few. Bring them all back together.
      39a72c16
    • Simon Tatham's avatar
      f5df09ad
    • Simon Tatham's avatar
      winpgnt: add context help for 'Add Key (encrypted)' button. · 9e3d78bd
      Simon Tatham authored
      I wrote a docs section, but forgot to link it to the context help.
      9e3d78bd
    • Simon Tatham's avatar
      pageant.rc: make a header file of dialog/control ids. · 0f61291f
      Simon Tatham authored
      I'm tired of remembering all those fiddly magic numbers and copying
      them back and forth between the .rc file and the source code. I'm even
      more tired of having to remember that in the long string of numbers
      after a dialog item definition, the first one of them _isn't_ one of
      the position and size coordinates. I've given them all symbolic names,
      like they should have had all along.
      
      I think I originally didn't bother because this was such a small GUI
      compared to the much larger one in PuTTY proper. But it's growing!
      0f61291f
    • Simon Tatham's avatar
      Windows Pageant: add --keylist option. · 44c084f3
      Simon Tatham authored
      This causes the main key list window to open when Pageant starts up,
      instead of waiting until you select 'View Keys' from the systray menu.
      
      My main motivation for adding this option is for development: if I'm
      _working_ on some detail of the key list window, it cuts down
      keystrokes in my edit-compile-retry cycle if I can have it
      automatically pop up in every new test run of Pageant.
      
      Normally I'd solve that by hacking an extra couple of lines
      temporarily into the code while I was doing that piece of development.
      But it suddenly struck me that there's no reason _not_ to add an
      option like this permanently (the space of word-length command-line
      flags is huge, and that particular one is unlikely to be needed for a
      different meaning), and who knows, it _might_ come in useful to
      someone in normal use. And at the very least it'll save me doing
      another temporary hack the next time I'm doing development work on the
      Pageant GUI. So I'll leave it in.
      44c084f3
  8. Apr 03, 2021
    • Simon Tatham's avatar
      Unix Pageant: revise --encrypted and -E CLI options. · c1334f3b
      Simon Tatham authored
      I've decided that it was a mistake to use -E as the option for adding
      keys encrypted, because it's better to use it as a fingerprint type
      selector for the Pageant client side. That way it works the same as
      command-line PuTTYgen, and also OpenSSH ssh-add (and ssh-keygen).
      
      What spelling(s) to use instead for the option to add keys encrypted?
      Obviously, the same ones I've just decided on for Windows Pageant;
      there's no sensible reason to make them different.
      c1334f3b
Loading