Is there a penalty for using BINARY(16) instead of UNIQUEIDENTIFIER?
Should I be concerned?


Well, there are a couple of things here that are a little concerning.

First: while it is true that a UNIQUEIDENTIFIER (i.e. Guid) is a 16-byte binary value, it is also true that:


All data can be stored in binary form (e.g. INT could be stored in BINARY(4), DATETIME can be stored in BINARY(8), etc), hence #2 ↴
There is probably a reason for having a separate datatype for GUIDs outside of mere convenience (e.g. sysname as an alias for NVARCHAR(128)).


The three behavioral differences that I can find are:


Comparing UNIQUEIDENTIFIER values in SQL Server, for better or for worse, is not actually done the same way as comparing BINARY(16) values. According to the MSDN page for Comparing GUID and uniqueidentifier Values, when comparing UNIQUEIDENTIFIER values in SQL Server:


  the last six bytes of a value are most significant

While these values are not frequently sorted, there is a slight difference between these two types. According to the MSDN page for uniqueidentifier:


  ordering is not implemented by comparing the bit patterns of the two values.

Given that there are differences in how GUID values are handled between SQL Server and .NET (noted in the "Comparing GUID and uniqueidentifier Values" page linked above), pulling this data out of SQL Server into app code might not be dealt with properly in the app code if needing to emulate the SQL Server comparison behavior. That behavior can be emulated by converting to a SqlGuid, but would a developer know to do that?


Second: based on the following statement


  It does this for everything including primary keys.


I would be concerned in general for system performance by using GUIDs as PKs instead of as Alternate Keys along with using an INT or even BIGINT as the PK. And even more concerned if these GUID PKs are the Clustered Indexes.

UPDATE

The following comment, made by the O.P. on @Rob's answer, brings up an additional concern:


  it was migrated from I think MySQL


GUIDs can be stored in 2 different binary formats. So, there could be cause for concern depending on:


what system the binary representation was generated on, and
if the string values were used outside of the original system, such as in app code or given to clients to use in import files, etc.


The issue with where the binary representation was generated has to do with the byte ordering of the first 3 out of the 4 "fields". If you follow the link above to the Wikipedia article, you will see that RFC 4122 specifies to use "Big Endian" encoding for all 4 fields, yet Microsoft GUIDs specify using "Native" Endianness. Well, Intel architecture is Little Endian, hence the byte order for the first 3 fields is reversed from systems following the RFC (as well as Microsoft-style GUIDs generated on Big Endian systems). The first field, "Data 1", is 4 bytes. In one Endianness it would be represented as (hypothetically) 0x01020304. But in the other Endianness it would be 0x04030201. So if the current database's BINARY(16) field was populated from an import file using 0x01020304 binary notation and that binary representation was generated on a system following the RFC, then converting the data currently in the BINARY(16) field into a UNIQUEIDENTIFIER will result in a different GUID than what was originally created. This does not really pose a problem IF the values never left the database, and the values are only ever compared for equality and not ordering.

The concern with ordering is simply that they won't be in the same order after converting to UNIQUEIDENTIFIER. Fortunately, if the original system really was MySQL then ordering was never done on the binary representation in the first place since MySQL only has a string representation of UUID.

The concern with the string values being used outside of the database is more serious, again, if the binary representation was generated outside of Windows / SQL Server. Since the byte ordering is potentially different, then the same GUID in string form would result in 2 different binary representations, depending on where that conversion took place. If app code or customers were given a GUID in string form as ABC coming from a binary form of 123 and the binary representation was generated on a system following the RFC, then that same binary representation (i.e. 123) would translate to a string form of DEF when converted to a UNIQUEIDENTIFIER. Likewise, the original string form of ABC would convert to a binary form of 456 when converted to a UNIQUEIDENTIFIER.

So, if the GUIDs never left the database then there isn't much to be concerned about outside of ordering. Or, if the import from MySQL was done by converting the string form (i.e. FCCEC3D8-22A0-4C8A-BF35-EC18227C9F40) then it might be ok. Else, if those GUIDs were given to customers or in the app code, you can test to see how they convert by getting one and converting via SELECT CONVERT(UNIQUEIDENTIFIER, 'value found outside of the database'); and see if you find the expected record. If you can't match records then you might have to keep the fields as BINARY(16).

In all likelihood there won't be an issue, but I am mentioning this because under the right conditions there could be an issue.

And how do new GUIDs get inserted anyway? Generated in the app code?

UPDATE 2

If the previous explanation of the potential issue related to importing binary representations of GUID generated on another system was a little (or a lot) confusing, hopefully the following will be a little clearer:

DECLARE @GUID UNIQUEIDENTIFIER = NEWID();
SELECT @GUID AS [String], CONVERT(BINARY(16), @GUID) AS [Binary];
-- String = 5FED23BE-E52C-40EE-8F45-49664C9472FD
-- Binary = 0xBE23ED5F2CE5EE408F4549664C9472FD
--          BE23ED5F-2CE5-EE40-8F45-49664C9472FD


In the output shown above, the "String" and "Binary" values are from the same GUID. The value beneath the "Binary" line is the same value as the "Binary" line, but formatted in the same style as the "String" line (i.e. removed "0x" and added the four dashes). Comparing the first and third values, they aren't exactly the same, but they are very close: the right-most two sections are identical, but the left-most three sections are not. But if you look closely, you can see that it is the same bytes in each of the three sections, just in a different order. It might be easier to see if I show only those first three sections, and number the bytes so it is easier to see how their order differs between the two representations:

String &nbsp;=  15F2ED3234BE &ndash; 5E562C &ndash; 7408EE
Binary = 4BE3232ED15F &ndash; 62C5E5 &ndash; 8EE740 (in Windows / SQL Server)

So within each grouping, the ordering of the bytes is reversed, but only within Windows and also SQL Server. However, on a system that adheres to the RFC, the binary representation would mirror the sting representation because there would not be any reversal of the byte order.

How was the data brought into SQL Server from MySQL? Here are a few choices:

SELECT CONVERT(BINARY(16), '5FED23BE-E52C-40EE-8F45-49664C9472FD'),
       CONVERT(BINARY(16), 0x5FED23BEE52C40EE8F4549664C9472FD),
    CONVERT(BINARY(16), CONVERT(UNIQUEIDENTIFIER, '5FED23BE-E52C-40EE-8F45-49664C9472FD'));


Returns:

0x35464544323342452D453532432D3430  
0x5FED23BEE52C40EE8F4549664C9472FD  
0xBE23ED5F2CE5EE408F4549664C9472FD


Assuming it was straight binary-to-binary (i.e. Convert #2 above), then the resulting GUID, if converted to an actual UNIQUEIDENTIFIER, would be:

SELECT CONVERT(UNIQUEIDENTIFIER, 0x5FED23BEE52C40EE8F4549664C9472FD);


Returns:

BE23ED5F-2CE5-EE40-8F45-49664C9472FD


Which is wrong. And that leaves us with three questions:


How was the data imported into SQL Server?
What language is the app code written in?
What platform is the app code running on?